Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove settings from AbstractAnalytic #152

Merged
merged 26 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8908212
remove settings from AbstractAnalytic
Rayus7 Mar 4, 2024
ba15bd3
update to new abstract-analytic plugin
Rayus7 Mar 4, 2024
c61c3b7
fix unit test for GoogleAnalytics4
Rayus7 Mar 4, 2024
7d30232
polishing changelog
Rayus7 Mar 4, 2024
63ddc25
fix name of plugin in changelog
Rayus7 Mar 4, 2024
b1412f8
update changelog
Rayus7 Mar 4, 2024
dbc94c9
fix name in changelog
Rayus7 Mar 4, 2024
def1f73
remove defaultDependencies altogether
Rayus7 Mar 5, 2024
5687eff
fix unit tests
Rayus7 Mar 5, 2024
1ae1543
make config property private
Rayus7 Mar 6, 2024
1230019
FacebookPixelAnalytic to TS
Rayus7 Mar 7, 2024
44e60f9
fix AbstractAnalytic unit tests
Rayus7 Mar 7, 2024
2c13c32
GoogleAnalytics4 Typescriptization, remove JSDOC type comments
Rayus7 Mar 8, 2024
ea6119c
Gtag type from DefinitelyTyped, fix unit tests
Rayus7 Mar 8, 2024
7845eeb
make other methods of AbstractAnalytic really abstract by typescript'…
Rayus7 Mar 8, 2024
36d5e85
fix unit test
Rayus7 Mar 8, 2024
8d6d2ab
purposeConsents is optional
Rayus7 Mar 11, 2024
efb0c98
whole initConfig for init mehthod is optional
Rayus7 Mar 11, 2024
b623131
CR fixes
Rayus7 Mar 12, 2024
9bebea8
Make properties and methods private if it makes sense
Rayus7 Mar 26, 2024
94a19fc
merge master to cnb-1172-remove-config
Rayus7 Mar 28, 2024
857edef
fix lint
Rayus7 Mar 28, 2024
8fd427b
fix import
Rayus7 Mar 28, 2024
0c17b3a
CR fixes
Rayus7 Apr 2, 2024
cc7c436
fix type
Rayus7 Apr 2, 2024
cfdb4a6
FacebookPixelAnalytic: remove public getter on config
Rayus7 Apr 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/[email protected]** 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
Loading