-
Notifications
You must be signed in to change notification settings - Fork 4
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #152 from seznam/cnb-1172-remove-config
remove settings from AbstractAnalytic
- Loading branch information
Showing
19 changed files
with
637 additions
and
497 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
--- | ||
"@ima/plugin-analytic-fb-pixel": major | ||
"@ima/plugin-analytic-google": major | ||
--- | ||
|
||
Update to new version of @ima/plugin-analytic | ||
|
||
- **What?** | ||
- Update to new version of [@ima/plugin-analytic](https://github.com/seznam/IMA.js-plugins/tree/master/packages/plugin-analytic), which doesn't save `config` argument to class variable anymore. | ||
- Config was moved to first position in dependencies list | ||
- Removed `defaultDependencies` export. | ||
- Typescriptization | ||
- Property `_fbq` is now protected (`#fbq`). | ||
- Removed property `_id` as it was not used anywhere. | ||
- **Why?** | ||
- Adding dependencies to subclasses is easier (no need to copy all dependencies, more info in @ima/plugin-analytic [CHANGELOG](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) | ||
- `defaultDependencies` was weird pattern, and we want to get rid of it | ||
- **How?** | ||
- If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. | ||
- Replace use of `defaultDependencies` by `$dependencies` property of given class plugin class. | ||
- Replace `_fbq` by `#fbq`. | ||
|
||
**!!** Use only with **@ima/plugin-analytic@6.0.0** or newer. **!!** |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
--- | ||
"@ima/plugin-analytic": major | ||
--- | ||
|
||
Removed config from constructor of `AbstractAnalytic` | ||
|
||
- **What?** | ||
- Removed `defaultDependencies` from plugin. | ||
- Removed config from constructor of `AbstractAnalytic` | ||
- Properties `_loaded`, `_scriptLoader`, `_dispatcher` and method `_afterLoadCallback` are now protected. | ||
(`#loaded`, `#scriptLoader`, `#dispatcher`, `#afterLoadCallback`) | ||
- New method `_isLoaded`. | ||
- **Why?** | ||
- `defaultDependencies` was weird pattern, and we want to get rid of it | ||
- To be able to use spread operator for dependencies in constructor of classes which extends `AbstractAnalytic`. | ||
Until now, we had to repeat all arguments from `AbstractAnalytic` constructor if we wanted to access `config` parameter, which is very common use-case. | ||
Also, now we can work with types in TypeScript more easily. | ||
- To clear the interface of `AbstractAnalytic`. | ||
- **How?** | ||
- Replace use of `defaultDependencies` by `AbstractAnalytic.$dependencies` | ||
- Classes, which extends `AbstractAnalytic` needs to save given config argument on their own. | ||
But you can use rest operator now. | ||
|
||
Therefore, this: | ||
```javascript | ||
class MyClass extends AbstractAnalytic { | ||
// Even here we were forced to copy dependencies from AbstractAnalytic to specify settings (last value in the array) | ||
static get $dependencies() { | ||
return [ | ||
NonAbstractAnalyticParam, | ||
ScriptLoaderPlugin, | ||
'$Window', | ||
'$Dispatcher', | ||
'$Settings.plugin.analytic.myClass', | ||
]; | ||
} | ||
constructor(nonAbstractAnalyticParam, scriptLoader, window, dispatcher, config) { | ||
super(scriptLoader, window, dispatcher, config); | ||
this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; | ||
this._id = config.id; // due to this line we were forced to copy all arguments of AbstractAnalytic | ||
// ... | ||
} | ||
} | ||
``` | ||
...can be rewritten to this: | ||
```javascript | ||
class MyClass extends AbstractAnalytic { | ||
// now we can define only added dependencies and use spread for the rest | ||
static get $dependencies() { | ||
return [ | ||
NonAbstractAnalyticParam, | ||
'$Settings.plugin.analytic.myClass', | ||
...AbstractAnalytic.$dependencies | ||
]; | ||
} | ||
constructor(nonAbstractAnalyticParam, config, ...rest) { | ||
super(...rest); | ||
this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; | ||
this._config = config; | ||
this._id = config.id; | ||
// ... | ||
} | ||
} | ||
``` | ||
- Replace use of `_scriptLoader`, `_dispatcher` and `_afterLoadCallback` to `#scriptLoader`, `#dispatcher` and `#afterLoadCallback`. | ||
Check if script is loaded by calling new method `_isLoaded()`. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
235 changes: 0 additions & 235 deletions
235
packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.