Skip to content

Commit

Permalink
Merge pull request #152 from seznam/cnb-1172-remove-config
Browse files Browse the repository at this point in the history
remove settings from AbstractAnalytic
  • Loading branch information
Rayus7 authored Apr 4, 2024
2 parents 6c654a1 + cfdb4a6 commit 66ef0fa
Show file tree
Hide file tree
Showing 19 changed files with 637 additions and 497 deletions.
23 changes: 23 additions & 0 deletions .changeset/few-yaks-scream.md
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. **!!**
75 changes: 75 additions & 0 deletions .changeset/pink-coats-rest.md
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()`.
17 changes: 16 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions packages/plugin-analytic-fb-pixel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,8 @@
"peerDependencies": {
"@ima/core": ">=18.0.0",
"@ima/plugin-script-loader": ">=3.1.1"
},
"devDependencies": {
"@types/facebook-pixel": "^0.0.30"
}
}
235 changes: 0 additions & 235 deletions packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js

This file was deleted.

Loading

0 comments on commit 66ef0fa

Please sign in to comment.