From 89082128ea8428787a828744319646e5e832abc5 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 11:32:11 +0100 Subject: [PATCH 01/25] remove settings from AbstractAnalytic --- .changeset/pink-coats-rest.md | 63 +++++++++++++++++++ .../plugin-analytic/src/AbstractAnalytic.ts | 9 +-- 2 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 .changeset/pink-coats-rest.md diff --git a/.changeset/pink-coats-rest.md b/.changeset/pink-coats-rest.md new file mode 100644 index 00000000..259cb1b6 --- /dev/null +++ b/.changeset/pink-coats-rest.md @@ -0,0 +1,63 @@ +--- +"@ima/plugin-analytic": major +--- + +Removed config from constructor of `AbstractAnalytic` + +- **What?** Removed config from constructor of `AbstractAnalytic` +- **Why?** 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. +- **How?** 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; + + // ... + } + } + ``` \ No newline at end of file diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index 347dfa45..b79bb17f 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -4,9 +4,6 @@ import { ScriptLoaderPlugin } from '@ima/plugin-script-loader'; import { Events as AnalyticEvents } from './Events'; -// eslint-disable-next-line @typescript-eslint/no-empty-interface -export interface AbstractAnalyticSettings {} - // @property purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel export type InitConfig = Record & { purposeConsents: Record; @@ -19,7 +16,6 @@ export default abstract class AbstractAnalytic { _scriptLoader: ScriptLoaderPlugin; _window: Window; _dispatcher: Dispatcher; - _config: AbstractAnalyticSettings; _analyticScriptName: string | null = null; //Analytic script url. _analyticScriptUrl: string | null = null; @@ -35,16 +31,13 @@ export default abstract class AbstractAnalytic { constructor( scriptLoader: ScriptLoaderPlugin, window: Window, - dispatcher: Dispatcher, - config: AbstractAnalyticSettings + dispatcher: Dispatcher ) { this._scriptLoader = scriptLoader; this._window = window; this._dispatcher = dispatcher; - - this._config = config; } /** From ba15bd36a87673dfb27ecf4600f73bde8ab749c5 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 14:10:46 +0100 Subject: [PATCH 02/25] update to new abstract-analytic plugin --- .changeset/few-yaks-scream.md | 12 ++++++++++ .../src/FacebookPixelAnalytic.js | 22 ++++++++++--------- .../src/GoogleAnalytics4.js | 16 ++++++++------ 3 files changed, 33 insertions(+), 17 deletions(-) create mode 100644 .changeset/few-yaks-scream.md diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md new file mode 100644 index 00000000..040cc423 --- /dev/null +++ b/.changeset/few-yaks-scream.md @@ -0,0 +1,12 @@ +--- +"@ima/plugin-analytic-fb-pixel": major +"@ima/plugin-analytic-google": major +--- + +Update to new version of @cns/plugin-analytic + +- **What?** + - Update to new version of [@cns/plugin-analytic](https://github.com/seznam/IMA.js-plugins/tree/master/packages/plugin-analytic), which requires to save `config` argument to class variable. + - Config is now first in dependencies list +- **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info [here](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) +- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`, diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js index 7e37ee47..b83e59c3 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js @@ -1,4 +1,4 @@ -import { AbstractAnalytic, defaultDependencies } from '@ima/plugin-analytic'; +import { AbstractAnalytic } from '@ima/plugin-analytic'; const FB_ROOT_VARIABLE = 'fbq'; @@ -10,24 +10,26 @@ const FB_ROOT_VARIABLE = 'fbq'; export default class FacebookPixelAnalytic extends AbstractAnalytic { /** @type {import('@ima/core').Dependencies} */ static get $dependencies() { - return [...defaultDependencies, '$Settings.plugin.analytic.fbPixel']; + return [ + '$Settings.plugin.analytic.fbPixel', + ...AbstractAnalytic.$dependencies, + ]; } /** * Creates a Facebook Pixel Helper instance. * * @function Object() { [native code] } - * @param {import('@ima/plugin-script-loader').ScriptLoaderPlugin} scriptLoader - * @param {import('@ima/core').Window} window - * @param {import('@ima/core').Dispatcher} dispatcher * @param {object} config */ - constructor(scriptLoader, window, dispatcher, config) { - super(scriptLoader, window, dispatcher, config); + constructor(config, ...rest) { + super(...rest); this._analyticScriptName = 'fb_pixel'; this._analyticScriptUrl = '//connect.facebook.net/en_US/fbevents.js'; + this._config = config; + /** * An identifier for Facebook Pixel. * @@ -66,7 +68,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * * @override * @param {string} eventName Name of the event. - * @param {object} [eventData=null] Data attached to the event. + * @param {object} [eventData] Data attached to the event. * @returns {boolean} TRUE when event has been hit; otherwise FALSE. */ hit(eventName, eventData = null) { @@ -103,7 +105,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * Hits a page view event (optionally with page view data). * * @override - * @param {object} [viewContentData = null] Page view data (containing path etc.). + * @param {object} [viewContentData] Page view data (containing path etc.). * @returns {boolean} TRUE when event has been hit; otherwise FALSE. */ hitPageView(viewContentData = null) { @@ -137,7 +139,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { /** * Hits a search event (optionally with page name or other event data). * - * @param {string|object} [queryOrData=null] Search query / event data. + * @param {string|object} [queryOrData] Search query / event data. * @returns {boolean} TRUE when event has been hit; otherwise FALSE. */ hitSearch(queryOrData = null) { diff --git a/packages/plugin-analytic-google/src/GoogleAnalytics4.js b/packages/plugin-analytic-google/src/GoogleAnalytics4.js index 85576211..e5180a0e 100644 --- a/packages/plugin-analytic-google/src/GoogleAnalytics4.js +++ b/packages/plugin-analytic-google/src/GoogleAnalytics4.js @@ -1,4 +1,4 @@ -import { AbstractAnalytic, defaultDependencies } from '@ima/plugin-analytic'; +import { AbstractAnalytic } from '@ima/plugin-analytic'; const GTAG_ROOT_VARIABLE = 'gtag'; @@ -8,7 +8,10 @@ const GTAG_ROOT_VARIABLE = 'gtag'; export default class GoogleAnalytics4 extends AbstractAnalytic { /** @type {import('@ima/core').Dependencies} */ static get $dependencies() { - return [...defaultDependencies, '$Settings.plugin.analytic.google4']; + return [ + '$Settings.plugin.analytic.google4', + ...AbstractAnalytic.$dependencies, + ]; } set _ga4Script(value) { @@ -26,13 +29,12 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { /** * Initializes the Google Analytics 4 plugin. * - * @param {import('@ima/plugin-script-loader').ScriptLoaderPlugin} scriptLoader - * @param {import('@ima/core').Window} window - * @param {import('@ima/core').Dispatcher} dispatcher * @param {Object} config */ - constructor(scriptLoader, window, dispatcher, config) { - super(scriptLoader, window, dispatcher, config); + constructor(config, ...rest) { + super(...rest); + + this._config = config; this._analyticScriptName = 'google_analytics_4'; From c61c3b7fd5f0629dfaf1d4bca8119d3be3b759b9 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 14:19:21 +0100 Subject: [PATCH 03/25] fix unit test for GoogleAnalytics4 --- .../src/__tests__/GoogleAnalytics4Spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js b/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js index 0627ac07..07d562f8 100644 --- a/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js +++ b/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js @@ -29,10 +29,10 @@ describe('GoogleAnalytics4', () => { beforeEach(() => { googleAnalytics4 = new GoogleAnalytics4( + settings, scriptLoader, window, - dispatcher, - settings + dispatcher ); }); From 7d302322bba683b6e5bc882fb9be71def55b4fc1 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 14:40:33 +0100 Subject: [PATCH 04/25] polishing changelog --- .changeset/few-yaks-scream.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index 040cc423..64340ce4 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -6,7 +6,7 @@ Update to new version of @cns/plugin-analytic - **What?** - - Update to new version of [@cns/plugin-analytic](https://github.com/seznam/IMA.js-plugins/tree/master/packages/plugin-analytic), which requires to save `config` argument to class variable. - - Config is now first in dependencies list -- **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info [here](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) -- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`, + - Update to new version of [@cns/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 +- **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info in @cns/plugin-analytic [CHANGELOG](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) +- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. From 63ddc250517e2f492e95cb18ce43a2514b428492 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 14:46:45 +0100 Subject: [PATCH 05/25] fix name of plugin in changelog --- .changeset/few-yaks-scream.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index 64340ce4..650c3533 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -3,10 +3,11 @@ "@ima/plugin-analytic-google": major --- -Update to new version of @cns/plugin-analytic +Update to new version of @ima/plugin-analytic - **What?** - - Update to new version of [@cns/plugin-analytic](https://github.com/seznam/IMA.js-plugins/tree/master/packages/plugin-analytic), which doesn't save `config` argument to class variable anymore. + - 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 - **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info in @cns/plugin-analytic [CHANGELOG](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) - **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. +Use only with **@ima/plugin-analytic@6.0.0** or newer. From b1412f8584bb8d8d098ff7c600aaee808ac11539 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 15:42:54 +0100 Subject: [PATCH 06/25] update changelog --- .changeset/few-yaks-scream.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index 650c3533..3a18b175 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -9,5 +9,6 @@ Update to new version of @ima/plugin-analytic - 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 - **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info in @cns/plugin-analytic [CHANGELOG](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) -- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. -Use only with **@ima/plugin-analytic@6.0.0** or newer. +- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. + + **!!** Use only with **@ima/plugin-analytic@6.0.0** or newer. **!!** From dbc94c9aad2a3ecdad07cb90378511b9bf802a3d Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 4 Mar 2024 15:44:15 +0100 Subject: [PATCH 07/25] fix name in changelog --- .changeset/few-yaks-scream.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index 3a18b175..c0b79c81 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -8,7 +8,7 @@ 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 -- **Why?** Adding dependencies to subclasses is easier (no need to copy all dependencies, more info in @cns/plugin-analytic [CHANGELOG](https://github.com/seznam/IMA.js-plugins/blob/master/packages/plugin-analytic/CHANGELOG.md#600)) +- **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)) - **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. **!!** Use only with **@ima/plugin-analytic@6.0.0** or newer. **!!** From def1f73de3fd27cd039b83637261ec2d16d93643 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 5 Mar 2024 13:35:15 +0100 Subject: [PATCH 08/25] remove defaultDependencies altogether --- .changeset/few-yaks-scream.md | 9 +- .changeset/pink-coats-rest.md | 100 ++++++++++-------- packages/plugin-analytic-fb-pixel/src/main.ts | 4 +- packages/plugin-analytic-google/src/main.ts | 4 +- packages/plugin-analytic/src/main.ts | 9 +- 5 files changed, 63 insertions(+), 63 deletions(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index c0b79c81..36c3692f 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -8,7 +8,12 @@ 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 -- **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)) -- **How?** If you extend `FacebookPixelAnalytic` or `GoogleAnalytics4` you need to move `config` parameter to the first position, when calling its `constructor`. + - Removed `defaultDependencies` export. +- **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. **!!** Use only with **@ima/plugin-analytic@6.0.0** or newer. **!!** diff --git a/.changeset/pink-coats-rest.md b/.changeset/pink-coats-rest.md index 259cb1b6..078ba0aa 100644 --- a/.changeset/pink-coats-rest.md +++ b/.changeset/pink-coats-rest.md @@ -4,60 +4,66 @@ Removed config from constructor of `AbstractAnalytic` -- **What?** Removed config from constructor of `AbstractAnalytic` -- **Why?** 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. -- **How?** Classes, which extends `AbstractAnalytic` needs to save given config argument on their own. -But you can use rest operator now. +- **What?** + - Removed `defaultDependencies` from plugin. + - Removed config from constructor of `AbstractAnalytic` +- **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. +- **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', - ]; - } + 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); + constructor(nonAbstractAnalyticParam, scriptLoader, window, dispatcher, config) { + super(scriptLoader, window, dispatcher, config); - this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; + this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; - this._id = config.id; // due to this line we were forced to copy all arguments of AbstractAnalytic + 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 - ]; - } + // ... + } + } + ``` + ...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); + constructor(nonAbstractAnalyticParam, config, ...rest) { + super(...rest); - this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; + this._nonAbstractAnalyticParam = nonAbstractAnalyticParam; - this._config = config; + this._config = config; - this._id = config.id; + this._id = config.id; - // ... - } - } - ``` \ No newline at end of file + // ... + } + } + ``` \ No newline at end of file diff --git a/packages/plugin-analytic-fb-pixel/src/main.ts b/packages/plugin-analytic-fb-pixel/src/main.ts index 02b898a6..5b4bb63a 100644 --- a/packages/plugin-analytic-fb-pixel/src/main.ts +++ b/packages/plugin-analytic-fb-pixel/src/main.ts @@ -21,8 +21,6 @@ declare module '@ima/core' { } } -const defaultDependencies = FacebookPixelAnalytic.$dependencies; - pluginLoader.register('@ima/plugin-analytic-google', () => ({ initSettings: () => ({ prod: { @@ -37,4 +35,4 @@ pluginLoader.register('@ima/plugin-analytic-google', () => ({ }), })); -export { FacebookPixelAnalytic, defaultDependencies }; +export { FacebookPixelAnalytic }; diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index d799e7d4..4221dc11 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -3,8 +3,6 @@ import { pluginLoader } from '@ima/core'; import GoogleAnalytics4 from './GoogleAnalytics4.js'; -const googleAnalytics4DefaultDependencies = GoogleAnalytics4.$dependencies; - export interface PluginAnalyticGoogleSettings { google4: { consentSettings?: { @@ -49,4 +47,4 @@ pluginLoader.register('@ima/plugin-analytic-google', () => ({ }), })); -export { GoogleAnalytics4, googleAnalytics4DefaultDependencies }; +export { GoogleAnalytics4 }; diff --git a/packages/plugin-analytic/src/main.ts b/packages/plugin-analytic/src/main.ts index 46d01f23..6eec350d 100644 --- a/packages/plugin-analytic/src/main.ts +++ b/packages/plugin-analytic/src/main.ts @@ -3,8 +3,6 @@ import AbstractAnalytic, { } from './AbstractAnalytic'; import { Events } from './Events'; -const defaultDependencies = AbstractAnalytic.$dependencies; - declare module '@ima/core' { // eslint-disable-next-line @typescript-eslint/no-empty-interface interface PluginAnalyticSettings {} @@ -24,9 +22,4 @@ declare module '@ima/core' { } } -export { - Events, - AbstractAnalytic, - AbstractAnalyticInitConfig, - defaultDependencies, -}; +export { Events, AbstractAnalytic, AbstractAnalyticInitConfig }; From 5687effe812595460b12da9450538f94ed530efc Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 5 Mar 2024 14:46:18 +0100 Subject: [PATCH 09/25] fix unit tests --- packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js | 4 ---- packages/plugin-analytic-google/src/__tests__/mainSpec.js | 4 ---- 2 files changed, 8 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js b/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js index a7955173..254a06a9 100644 --- a/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js +++ b/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js @@ -4,8 +4,4 @@ describe('Main', () => { it('should export FacebookPixelAnalytic', () => { expect(typeof Main.FacebookPixelAnalytic).toBe('function'); }); - - it('should export defaultDependencies', () => { - expect(Array.isArray(Main.defaultDependencies)).toBe(true); - }); }); diff --git a/packages/plugin-analytic-google/src/__tests__/mainSpec.js b/packages/plugin-analytic-google/src/__tests__/mainSpec.js index a43e49aa..a228849c 100644 --- a/packages/plugin-analytic-google/src/__tests__/mainSpec.js +++ b/packages/plugin-analytic-google/src/__tests__/mainSpec.js @@ -4,8 +4,4 @@ describe('Main', () => { it('should export GoogleAnalytics4', () => { expect(typeof Main.GoogleAnalytics4).toBe('function'); }); - - it('should export googleAnalytics4DefaultDependencies', () => { - expect(Array.isArray(Main.googleAnalytics4DefaultDependencies)).toBe(true); - }); }); From 1ae1543ab5c09b2b457b1922a9d8b24c2e5ecfd1 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Wed, 6 Mar 2024 08:33:50 +0100 Subject: [PATCH 10/25] make config property private --- .../src/FacebookPixelAnalytic.js | 14 ++++++++++---- .../src/GoogleAnalytics4.js | 16 +++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js index b83e59c3..3084cb10 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js @@ -8,6 +8,8 @@ const FB_ROOT_VARIABLE = 'fbq'; * @class */ export default class FacebookPixelAnalytic extends AbstractAnalytic { + #config; + /** @type {import('@ima/core').Dependencies} */ static get $dependencies() { return [ @@ -16,6 +18,10 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { ]; } + get config() { + return this.#config; + } + /** * Creates a Facebook Pixel Helper instance. * @@ -28,7 +34,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { this._analyticScriptName = 'fb_pixel'; this._analyticScriptUrl = '//connect.facebook.net/en_US/fbevents.js'; - this._config = config; + this.#config = config; /** * An identifier for Facebook Pixel. @@ -51,11 +57,11 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * @returns {string} The identifier for Facebook Pixel. */ getId() { - switch (typeof this._config.id) { + switch (typeof this.config.id) { case 'number': - return String(this._config.id); + return String(this.config.id); case 'string': - return this._config.id; + return this.config.id; default: throw new TypeError( 'A Facebook Pixel identifier should be a number/string.' diff --git a/packages/plugin-analytic-google/src/GoogleAnalytics4.js b/packages/plugin-analytic-google/src/GoogleAnalytics4.js index e5180a0e..d5fba24b 100644 --- a/packages/plugin-analytic-google/src/GoogleAnalytics4.js +++ b/packages/plugin-analytic-google/src/GoogleAnalytics4.js @@ -6,6 +6,8 @@ const GTAG_ROOT_VARIABLE = 'gtag'; * Google analytic 4 class */ export default class GoogleAnalytics4 extends AbstractAnalytic { + #config; + /** @type {import('@ima/core').Dependencies} */ static get $dependencies() { return [ @@ -26,6 +28,10 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { return clientWindow[GTAG_ROOT_VARIABLE]; } + get config() { + return this.#config; + } + /** * Initializes the Google Analytics 4 plugin. * @@ -34,13 +40,13 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { constructor(config, ...rest) { super(...rest); - this._config = config; + this.#config = config; this._analyticScriptName = 'google_analytics_4'; - this._analyticScriptUrl = `https://www.googletagmanager.com/gtag/js?id=${this._config.service}`; + this._analyticScriptUrl = `https://www.googletagmanager.com/gtag/js?id=${this.config.service}`; - this._consentSettings = this._config.consentSettings; + this._consentSettings = this.config.consentSettings; } /** * Hits custom event of given with given data @@ -115,12 +121,12 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { this._ga4Script('consent', 'default', { ...this._consentSettings, - wait_for_update: this._config.waitForUpdateTimeout, + wait_for_update: this.config.waitForUpdateTimeout, }); this._ga4Script('js', new Date()); - this._ga4Script('config', this._config.service, { + this._ga4Script('config', this.config.service, { send_page_view: false, }); } From 1230019de31d38ba520f03b95b0cab37c4196229 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Thu, 7 Mar 2024 14:04:59 +0100 Subject: [PATCH 11/25] FacebookPixelAnalytic to TS --- package-lock.json | 9 ++ .../plugin-analytic-fb-pixel/package.json | 3 + ...elAnalytic.js => FacebookPixelAnalytic.ts} | 89 ++++++++++++------- packages/plugin-analytic-fb-pixel/src/main.ts | 17 +++- .../plugin-analytic/src/AbstractAnalytic.ts | 23 ++--- packages/plugin-analytic/src/main.ts | 9 +- 6 files changed, 97 insertions(+), 53 deletions(-) rename packages/plugin-analytic-fb-pixel/src/{FacebookPixelAnalytic.js => FacebookPixelAnalytic.ts} (69%) diff --git a/package-lock.json b/package-lock.json index 38a59aea..44d7c4b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4860,6 +4860,12 @@ "resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.5.tgz", "integrity": "sha512-/kYRxGDLWzHOB7q+wtSUQlFrtcdUccpfy+X+9iMBpHK8QLLhx2wIPYuS5DYtR9Wa/YlZAbIovy7qVdB1Aq6Lyw==" }, + "node_modules/@types/facebook-pixel": { + "version": "0.0.30", + "resolved": "https://registry.npmjs.org/@types/facebook-pixel/-/facebook-pixel-0.0.30.tgz", + "integrity": "sha512-zg/T6dmkcyzX4rj4MjnEUdijPjF7Fj1xw8CmYxz5MeoUuYU0iDbTL8qNdcbSTTkRrPTCtq3Db002Use4ikoNmw==", + "dev": true + }, "node_modules/@types/graceful-fs": { "version": "4.1.9", "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.9.tgz", @@ -22457,6 +22463,9 @@ "dependencies": { "@ima/plugin-analytic": "^5.0.2" }, + "devDependencies": { + "@types/facebook-pixel": "^0.0.30" + }, "peerDependencies": { "@ima/core": ">=18.0.0", "@ima/plugin-script-loader": ">=3.1.1" diff --git a/packages/plugin-analytic-fb-pixel/package.json b/packages/plugin-analytic-fb-pixel/package.json index 1eda017c..3fb42e77 100644 --- a/packages/plugin-analytic-fb-pixel/package.json +++ b/packages/plugin-analytic-fb-pixel/package.json @@ -39,5 +39,8 @@ "peerDependencies": { "@ima/core": ">=18.0.0", "@ima/plugin-script-loader": ">=3.1.1" + }, + "devDependencies": { + "@types/facebook-pixel": "^0.0.30" } } diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts similarity index 69% rename from packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js rename to packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index 3084cb10..b5dc6fa6 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.js +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -1,17 +1,25 @@ +import { Dependencies } from '@ima/core'; import { AbstractAnalytic } from '@ima/plugin-analytic'; const FB_ROOT_VARIABLE = 'fbq'; +export type AnalyticFBPixelSettings = { + id: string | null; +}; + /** * Facebook Pixel Helper. * * @class */ export default class FacebookPixelAnalytic extends AbstractAnalytic { - #config; + #config: AnalyticFBPixelSettings; + // An identifier for Facebook Pixel. + _id: string | null; + // A main function of Facebook Pixel. + _fbq: facebook.Pixel.Event | null; - /** @type {import('@ima/core').Dependencies} */ - static get $dependencies() { + static get $dependencies(): Dependencies { return [ '$Settings.plugin.analytic.fbPixel', ...AbstractAnalytic.$dependencies, @@ -26,9 +34,13 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * Creates a Facebook Pixel Helper instance. * * @function Object() { [native code] } + * @param {...any} rest * @param {object} config */ - constructor(config, ...rest) { + constructor( + config: AnalyticFBPixelSettings, + ...rest: ConstructorParameters + ) { super(...rest); this._analyticScriptName = 'fb_pixel'; @@ -36,18 +48,8 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { this.#config = config; - /** - * An identifier for Facebook Pixel. - * - * @type {string} - */ this._id = null; - /** - * A main function of Facebook Pixel. - * - * @type {Function} - */ this._fbq = null; } @@ -73,11 +75,11 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * Hits an event. * * @override - * @param {string} eventName Name of the event. - * @param {object} [eventData] Data attached to the event. - * @returns {boolean} TRUE when event has been hit; otherwise FALSE. + * @param eventName Name of the event. + * @param eventData Data attached to the event. + * @returns TRUE when event has been hit; otherwise FALSE. */ - hit(eventName, eventData = null) { + hit(eventName: string, eventData: object | null = null) { try { if (!this._fbq) { throw new Error( @@ -93,7 +95,9 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { ); } } catch (error) { - this._processError(error); + this._processError( + error as Parameters[0] + ); return false; } @@ -126,12 +130,14 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { ); } } catch (error) { - this._processError(error); + this._processError( + error as Parameters[0] + ); return false; } - let hitResult = this.hit('PageView'); + const hitResult = this.hit('PageView'); if (!hitResult) { return false; @@ -160,7 +166,9 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { ); } } catch (error) { - this._processError(error); + this._processError( + error as Parameters[0] + ); return false; } @@ -185,7 +193,8 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * @inheritdoc */ _configuration() { - const clientWindow = this._window.getWindow(); + // _configuration is only called on client, therefore window is defined + const clientWindow = this._window.getWindow()!; if ( this.isEnabled() || @@ -197,27 +206,39 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { this._enable = true; - this._fbq = window[FB_ROOT_VARIABLE]; - this._fbq('init', this.getId()); + this._fbq = clientWindow[FB_ROOT_VARIABLE]; + this._fbq!('init', this.getId()); } /** * @override * @inheritdoc */ - _createGlobalDefinition(window) { + _createGlobalDefinition(window: globalThis.Window) { if (window[FB_ROOT_VARIABLE]) { return; } - const fbAnalytic = (window[FB_ROOT_VARIABLE] = function () { + interface FbAnalytic { + callMethod: (...params: unknown[]) => void; + queue: any[]; + push: FbAnalytic; + loaded: boolean; + version: string; + } + + type FbAnalyiticExtended = facebook.Pixel.Event & FbAnalytic; + + const fbAnalytic = (window[FB_ROOT_VARIABLE] = function ( + ...rest: unknown[] + ) { fbAnalytic.callMethod - ? fbAnalytic.callMethod.apply(fbAnalytic, arguments) - : fbAnalytic.queue.push(arguments); - }); + ? fbAnalytic.callMethod(...rest) + : fbAnalytic.queue.push(...rest); + }) as FbAnalyiticExtended; - if (!window['_' + FB_ROOT_VARIABLE]) { - window['_' + FB_ROOT_VARIABLE] = fbAnalytic; + if (!window[`_${FB_ROOT_VARIABLE}`]) { + window[`_${FB_ROOT_VARIABLE}`] = fbAnalytic; } fbAnalytic.push = fbAnalytic; @@ -233,9 +254,9 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { /** * Processes an error. * - * @param {Error|TypeError|string} error An error to be processed. + * @param error An error to be processed. */ - _processError(error) { + _processError(error: Error | TypeError | string) { if ($Debug && error) { console.error(error); } diff --git a/packages/plugin-analytic-fb-pixel/src/main.ts b/packages/plugin-analytic-fb-pixel/src/main.ts index 5b4bb63a..16b1289d 100644 --- a/packages/plugin-analytic-fb-pixel/src/main.ts +++ b/packages/plugin-analytic-fb-pixel/src/main.ts @@ -1,6 +1,8 @@ import { pluginLoader } from '@ima/core'; -import FacebookPixelAnalytic from './FacebookPixelAnalytic'; +import FacebookPixelAnalytic, { + type AnalyticFBPixelSettings, +} from './FacebookPixelAnalytic'; export interface PluginAnalyticFBPixelSettings { fbPixel: { @@ -8,6 +10,13 @@ export interface PluginAnalyticFBPixelSettings { }; } +declare global { + interface Window { + fbq: facebook.Pixel.Event; + _fbq: facebook.Pixel.Event; + } +} + declare module '@ima/core' { // eslint-disable-next-line @typescript-eslint/no-empty-interface interface PluginAnalyticSettings extends PluginAnalyticFBPixelSettings {} @@ -19,6 +28,10 @@ declare module '@ima/core' { interface Settings { plugin: PluginSettings; } + + interface OCAliasMap { + '$Settings.plugin.analytic.fbPixel': AnalyticFBPixelSettings; + } } pluginLoader.register('@ima/plugin-analytic-google', () => ({ @@ -36,3 +49,5 @@ pluginLoader.register('@ima/plugin-analytic-google', () => ({ })); export { FacebookPixelAnalytic }; + +export type { AnalyticFBPixelSettings }; diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index b79bb17f..64089ae0 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -12,16 +12,16 @@ export type InitConfig = Record & { /** * Abstract analytic class */ -export default abstract class AbstractAnalytic { +export abstract class AbstractAnalytic { _scriptLoader: ScriptLoaderPlugin; _window: Window; _dispatcher: Dispatcher; _analyticScriptName: string | null = null; - //Analytic script url. + // Analytic script url. _analyticScriptUrl: string | null = null; - //If flag has value true then analytic is enabled to hit events. + // If flag has value true then analytic is enabled to hit events. _enable = false; - //If flag has value true then analytic script was loaded. + // If flag has value true then analytic script was loaded. _loaded = false; static get $dependencies(): Dependencies { @@ -49,7 +49,8 @@ export default abstract class AbstractAnalytic { */ init(initConfig: InitConfig) { if (!this.isEnabled() && this._window.isClient()) { - const window = this._window.getWindow() as globalThis.Window; + // we are on client, therefore window is defined + const window = this._window.getWindow()!; if (initConfig && initConfig.purposeConsents) { this._applyPurposeConsents(initConfig.purposeConsents); @@ -115,23 +116,15 @@ export default abstract class AbstractAnalytic { * defer hit to storage. * * @abstract - * @param _data */ - hit(_data: Record) { - throw new Error('The hit() method is abstract and must be overridden.'); - } + abstract hit(...args: unknown[]): void; /** * Hit page view event to analytic for defined page data. * * @abstract - * @param _pageData */ - hitPageView(_pageData: Record) { - throw new Error( - 'The hitPageView() method is abstract and must be overridden.' - ); - } + abstract hitPageView(...args: unknown[]): void; /** * Configuration analytic. The analytic must be enabled after configuration. diff --git a/packages/plugin-analytic/src/main.ts b/packages/plugin-analytic/src/main.ts index 6eec350d..f527dad3 100644 --- a/packages/plugin-analytic/src/main.ts +++ b/packages/plugin-analytic/src/main.ts @@ -1,5 +1,6 @@ -import AbstractAnalytic, { - InitConfig as AbstractAnalyticInitConfig, +import { + AbstractAnalytic, + type InitConfig as AbstractAnalyticInitConfig, } from './AbstractAnalytic'; import { Events } from './Events'; @@ -22,4 +23,6 @@ declare module '@ima/core' { } } -export { Events, AbstractAnalytic, AbstractAnalyticInitConfig }; +export { Events, AbstractAnalytic }; + +export type { AbstractAnalyticInitConfig }; From 44e60f93090679d4f438d179cf53cd7b72de66b3 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Thu, 7 Mar 2024 14:19:38 +0100 Subject: [PATCH 12/25] fix AbstractAnalytic unit tests --- .../plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts | 4 ++++ .../plugin-analytic/src/__tests__/AbstractAnalyticSpec.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index b5dc6fa6..f275bbb4 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -219,6 +219,10 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { return; } + /* + * Whole code of this method is implementation of initialization code of Facebook Pixel from their documentation. + * Not everything used here is typed on the internet (at least I do not find it), therefore it is typed by us here. + */ interface FbAnalytic { callMethod: (...params: unknown[]) => void; queue: any[]; diff --git a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js index d8d6368c..ce815f76 100644 --- a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js +++ b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js @@ -2,7 +2,7 @@ import { Window, Dispatcher } from '@ima/core'; import { ScriptLoaderPlugin } from '@ima/plugin-script-loader'; import { toMockedInstance } from 'to-mock'; -import AbstractAnalytic from '../AbstractAnalytic'; +import { AbstractAnalytic } from '../AbstractAnalytic'; import { Events as AnalyticEvents } from '../Events'; describe('AbstractAnalytic', () => { From 2c13c32a712e4c18b1e4d97623c9bb276c47fc9d Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Fri, 8 Mar 2024 09:30:43 +0100 Subject: [PATCH 13/25] GoogleAnalytics4 Typescriptization, remove JSDOC type comments --- .changeset/few-yaks-scream.md | 1 + .../src/FacebookPixelAnalytic.ts | 33 ++++---- packages/plugin-analytic-fb-pixel/src/main.ts | 15 ++-- ...oogleAnalytics4.js => GoogleAnalytics4.ts} | 79 +++++++++++-------- .../src/__tests__/GoogleAnalytics4Spec.js | 2 +- packages/plugin-analytic-google/src/main.ts | 28 ++++--- 6 files changed, 88 insertions(+), 70 deletions(-) rename packages/plugin-analytic-google/src/{GoogleAnalytics4.js => GoogleAnalytics4.ts} (54%) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index 36c3692f..ce7be409 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -9,6 +9,7 @@ Update to new version of @ima/plugin-analytic - 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 - **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 diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index f275bbb4..9830c305 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -12,7 +12,7 @@ export type AnalyticFBPixelSettings = { * * @class */ -export default class FacebookPixelAnalytic extends AbstractAnalytic { +export class FacebookPixelAnalytic extends AbstractAnalytic { #config: AnalyticFBPixelSettings; // An identifier for Facebook Pixel. _id: string | null; @@ -32,10 +32,8 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { /** * Creates a Facebook Pixel Helper instance. - * - * @function Object() { [native code] } + * @param config * @param {...any} rest - * @param {object} config */ constructor( config: AnalyticFBPixelSettings, @@ -56,7 +54,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { /** * Gets the identifier for Facebook Pixel. * - * @returns {string} The identifier for Facebook Pixel. + * @returns The identifier for Facebook Pixel. */ getId() { switch (typeof this.config.id) { @@ -79,7 +77,7 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * @param eventData Data attached to the event. * @returns TRUE when event has been hit; otherwise FALSE. */ - hit(eventName: string, eventData: object | null = null) { + hit(eventName: string, eventData: Record | null = null) { try { if (!this._fbq) { throw new Error( @@ -115,10 +113,10 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { * Hits a page view event (optionally with page view data). * * @override - * @param {object} [viewContentData] Page view data (containing path etc.). - * @returns {boolean} TRUE when event has been hit; otherwise FALSE. + * @param Page view data (containing path etc.). + * @returns TRUE when event has been hit; otherwise FALSE. */ - hitPageView(viewContentData = null) { + hitPageView(viewContentData: Record | null = null) { try { if (!this._fbq) { throw new Error( @@ -151,10 +149,11 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { /** * Hits a search event (optionally with page name or other event data). * - * @param {string|object} [queryOrData] Search query / event data. - * @returns {boolean} TRUE when event has been hit; otherwise FALSE. + * @param Search query / event data. + * @param queryOrData + * @returns TRUE when event has been hit; otherwise FALSE. */ - hitSearch(queryOrData = null) { + hitSearch(queryOrData: Record | string | null = null) { try { if (!this._fbq) { throw new Error( @@ -173,19 +172,15 @@ export default class FacebookPixelAnalytic extends AbstractAnalytic { return false; } - let eventData; + let eventData: Record | null; if (typeof queryOrData === 'string' && queryOrData) { eventData = { search_string: queryOrData }; } else { - eventData = queryOrData; - } - - if (!eventData) { return this.hit('Search'); - } else { - return this.hit('Search', eventData); } + + return this.hit('Search', eventData); } /** diff --git a/packages/plugin-analytic-fb-pixel/src/main.ts b/packages/plugin-analytic-fb-pixel/src/main.ts index 16b1289d..d8a42863 100644 --- a/packages/plugin-analytic-fb-pixel/src/main.ts +++ b/packages/plugin-analytic-fb-pixel/src/main.ts @@ -1,15 +1,10 @@ import { pluginLoader } from '@ima/core'; -import FacebookPixelAnalytic, { +import { + FacebookPixelAnalytic, type AnalyticFBPixelSettings, } from './FacebookPixelAnalytic'; -export interface PluginAnalyticFBPixelSettings { - fbPixel: { - id: string | null; - }; -} - declare global { interface Window { fbq: facebook.Pixel.Event; @@ -17,6 +12,12 @@ declare global { } } +export interface PluginAnalyticFBPixelSettings { + fbPixel: { + id: string | null; + }; +} + declare module '@ima/core' { // eslint-disable-next-line @typescript-eslint/no-empty-interface interface PluginAnalyticSettings extends PluginAnalyticFBPixelSettings {} diff --git a/packages/plugin-analytic-google/src/GoogleAnalytics4.js b/packages/plugin-analytic-google/src/GoogleAnalytics4.ts similarity index 54% rename from packages/plugin-analytic-google/src/GoogleAnalytics4.js rename to packages/plugin-analytic-google/src/GoogleAnalytics4.ts index d5fba24b..3d6fb1f5 100644 --- a/packages/plugin-analytic-google/src/GoogleAnalytics4.js +++ b/packages/plugin-analytic-google/src/GoogleAnalytics4.ts @@ -1,15 +1,28 @@ +import { Dependencies } from '@ima/core'; import { AbstractAnalytic } from '@ima/plugin-analytic'; const GTAG_ROOT_VARIABLE = 'gtag'; +type ConsentSettings = { + ad_storage?: 'denied' | 'granted'; + analytics_storage?: 'denied' | 'granted'; + personalization_storage?: 'denied' | 'granted'; +}; + +export type AnalyticGoogleSettings = { + consentSettings?: ConsentSettings; + service: string; + waitForUpdateTimeout?: number; +}; + /** * Google analytic 4 class */ -export default class GoogleAnalytics4 extends AbstractAnalytic { - #config; +export class GoogleAnalytics4 extends AbstractAnalytic { + #config: AnalyticGoogleSettings; + #consentSettings?: ConsentSettings; - /** @type {import('@ima/core').Dependencies} */ - static get $dependencies() { + static get $dependencies(): Dependencies { return [ '$Settings.plugin.analytic.google4', ...AbstractAnalytic.$dependencies, @@ -17,13 +30,13 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { } set _ga4Script(value) { - const clientWindow = this._window.getWindow(); + const clientWindow = this._window.getWindow()!; clientWindow[GTAG_ROOT_VARIABLE] = value; } get _ga4Script() { - const clientWindow = this._window.getWindow(); + const clientWindow = this._window.getWindow()!; return clientWindow[GTAG_ROOT_VARIABLE]; } @@ -34,10 +47,11 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { /** * Initializes the Google Analytics 4 plugin. - * - * @param {Object} config */ - constructor(config, ...rest) { + constructor( + config: AnalyticGoogleSettings, + ...rest: ConstructorParameters + ) { super(...rest); this.#config = config; @@ -46,15 +60,15 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { this._analyticScriptUrl = `https://www.googletagmanager.com/gtag/js?id=${this.config.service}`; - this._consentSettings = this.config.consentSettings; + this.#consentSettings = this.config.consentSettings; } /** * Hits custom event of given with given data * - * @param {string} eventName custom event name - * @param {Object} eventData custom event data + * @param eventName custom event name + * @param eventData custom event data */ - hit(eventName, eventData) { + hit(eventName: string, eventData: Record) { if (!this.isEnabled()) { return; } @@ -64,12 +78,9 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { /** * Hit page view event to analytic with defined data. - * - * @override - * @param {Object} pageData - * @param {Object} customDimensions + * @param pageData */ - hitPageView(pageData) { + hitPageView(pageData: Record) { if (!this.isEnabled()) { return; } @@ -80,13 +91,13 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { /** * Updates user consents in Google Analytics script * - * @param {Object} purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel + * @param purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel */ - updateConsent(purposeConsents) { + updateConsent(purposeConsents: Record) { this._applyPurposeConsents(purposeConsents); this._ga4Script('consent', 'update', { - ...this._consentSettings, + ...this.#consentSettings, }); } @@ -94,12 +105,16 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { * @override * @inheritdoc */ - _applyPurposeConsents(purposeConsents) { - if (purposeConsents && typeof purposeConsents === 'object') { + _applyPurposeConsents(purposeConsents: Record) { + if ( + purposeConsents && + typeof purposeConsents === 'object' && + this.#consentSettings + ) { if (purposeConsents['1']) { - this._consentSettings.analytics_storage = 'granted'; + this.#consentSettings.analytics_storage = 'granted'; } else { - this._consentSettings.analytics_storage = 'denied'; + this.#consentSettings.analytics_storage = 'denied'; } } } @@ -120,7 +135,7 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { this._enable = true; this._ga4Script('consent', 'default', { - ...this._consentSettings, + ...this.#consentSettings, wait_for_update: this.config.waitForUpdateTimeout, }); @@ -133,11 +148,8 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { /** * Returns page view data derived from pageData param. - * - * @param {Object} pageData - * @returns {Object} pageViewData */ - _getPageViewData(pageData) { + _getPageViewData(pageData: Record) { return { page: pageData.path, location: this._window.getUrl(), @@ -150,12 +162,13 @@ export default class GoogleAnalytics4 extends AbstractAnalytic { * @inheritdoc */ _createGlobalDefinition() { - const window = this._window.getWindow(); + // _createGlobalDefinition is called only on client, therefore window is defined + const window = this._window.getWindow()!; window.dataLayer = window.dataLayer || []; - this._ga4Script = function () { - window.dataLayer.push(arguments); + this._ga4Script = function (...rest: unknown[]) { + window.dataLayer.push(...rest); }; this._configuration(); diff --git a/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js b/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js index 07d562f8..7848effe 100644 --- a/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js +++ b/packages/plugin-analytic-google/src/__tests__/GoogleAnalytics4Spec.js @@ -2,7 +2,7 @@ import { Dispatcher, Window } from '@ima/core'; import { ScriptLoaderPlugin } from '@ima/plugin-script-loader'; import { toMockedInstance } from 'to-mock'; -import GoogleAnalytics4 from '../GoogleAnalytics4'; +import { GoogleAnalytics4 } from '../GoogleAnalytics4'; describe('GoogleAnalytics4', () => { const settings = { diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index 4221dc11..79754071 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -1,18 +1,20 @@ /* eslint-disable @typescript-eslint/no-empty-interface */ import { pluginLoader } from '@ima/core'; -import GoogleAnalytics4 from './GoogleAnalytics4.js'; +import { + GoogleAnalytics4, + type AnalyticGoogleSettings, +} from './GoogleAnalytics4.js'; + +declare global { + interface Window { + gtag: any; // TODO + dataLayer: unknown[]; + } +} export interface PluginAnalyticGoogleSettings { - google4: { - consentSettings?: { - ad_storage?: 'denied' | 'granted'; - analytics_storage?: 'denied' | 'granted'; - personalization_storage?: 'denied' | 'granted'; - }; - service: string; - waitForUpdateTimeout?: number; - }; + google4: AnalyticGoogleSettings; } declare module '@ima/core' { @@ -25,6 +27,10 @@ declare module '@ima/core' { interface Settings { plugin: PluginSettings; } + + interface OCAliasMap { + '$Settings.plugin.analytic.google4': AnalyticGoogleSettings; + } } pluginLoader.register('@ima/plugin-analytic-google', () => ({ @@ -48,3 +54,5 @@ pluginLoader.register('@ima/plugin-analytic-google', () => ({ })); export { GoogleAnalytics4 }; + +export type { AnalyticGoogleSettings }; From ea6119cc84e041704cd60fe7f13f85060b29f0ea Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Fri, 8 Mar 2024 12:06:44 +0100 Subject: [PATCH 14/25] Gtag type from DefinitelyTyped, fix unit tests --- package-lock.json | 9 +++++++++ .../plugin-analytic-fb-pixel/src/__tests__/mainSpec.js | 4 ++-- packages/plugin-analytic-google/package.json | 5 ++++- .../plugin-analytic-google/src/__tests__/mainSpec.js | 4 ++-- packages/plugin-analytic-google/src/main.ts | 2 +- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 44d7c4b4..f8245b89 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4875,6 +4875,12 @@ "@types/node": "*" } }, + "node_modules/@types/gtag.js": { + "version": "0.0.19", + "resolved": "https://registry.npmjs.org/@types/gtag.js/-/gtag.js-0.0.19.tgz", + "integrity": "sha512-KHoDzrf9rSd0mooKN576PjExpdk/XRrNu4RQnmigsScSTSidwyOUe9kDrHz9UPKjiBrx2QEsSkexbJSgS0j72w==", + "dev": true + }, "node_modules/@types/http-proxy": { "version": "1.17.14", "resolved": "https://registry.npmjs.org/@types/http-proxy/-/http-proxy-1.17.14.tgz", @@ -22479,6 +22485,9 @@ "@ima/plugin-analytic": "^5.0.2", "@ima/plugin-script-loader": "^3.1.1" }, + "devDependencies": { + "@types/gtag.js": "^0.0.19" + }, "peerDependencies": { "@ima/core": ">=18.0.0", "@ima/plugin-script-loader": ">=3.1.1" diff --git a/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js b/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js index 254a06a9..73290eef 100644 --- a/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js +++ b/packages/plugin-analytic-fb-pixel/src/__tests__/mainSpec.js @@ -1,7 +1,7 @@ -import * as Main from '../main'; +import { FacebookPixelAnalytic } from '../main'; describe('Main', () => { it('should export FacebookPixelAnalytic', () => { - expect(typeof Main.FacebookPixelAnalytic).toBe('function'); + expect(typeof FacebookPixelAnalytic).toBe('function'); }); }); diff --git a/packages/plugin-analytic-google/package.json b/packages/plugin-analytic-google/package.json index cc9a2b92..3681346f 100644 --- a/packages/plugin-analytic-google/package.json +++ b/packages/plugin-analytic-google/package.json @@ -34,10 +34,13 @@ "license": "MIT", "dependencies": { "@ima/plugin-analytic": "^5.0.2", - "@ima/plugin-script-loader": "^3.1.1" + "@ima/plugin-script-loader": ">=3.1.1" }, "peerDependencies": { "@ima/core": ">=18.0.0", "@ima/plugin-script-loader": ">=3.1.1" + }, + "devDependencies": { + "@types/gtag.js": "^0.0.19" } } diff --git a/packages/plugin-analytic-google/src/__tests__/mainSpec.js b/packages/plugin-analytic-google/src/__tests__/mainSpec.js index a228849c..e556b8fb 100644 --- a/packages/plugin-analytic-google/src/__tests__/mainSpec.js +++ b/packages/plugin-analytic-google/src/__tests__/mainSpec.js @@ -1,7 +1,7 @@ -import * as Main from '../main'; +import { GoogleAnalytics4 } from '../main'; describe('Main', () => { it('should export GoogleAnalytics4', () => { - expect(typeof Main.GoogleAnalytics4).toBe('function'); + expect(typeof GoogleAnalytics4).toBe('function'); }); }); diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index 79754071..39b6547c 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -8,7 +8,7 @@ import { declare global { interface Window { - gtag: any; // TODO + gtag: Gtag.Gtag; dataLayer: unknown[]; } } From 7845eeb53f39033dabce42a3f0f274b59dc5032d Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Fri, 8 Mar 2024 12:37:19 +0100 Subject: [PATCH 15/25] make other methods of AbstractAnalytic really abstract by typescript's 'abstract' keyword --- .../src/FacebookPixelAnalytic.ts | 4 ++++ .../src/GoogleAnalytics4.ts | 19 +++++++---------- packages/plugin-analytic-google/src/main.ts | 2 +- .../plugin-analytic/src/AbstractAnalytic.ts | 21 ++++--------------- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index 9830c305..39b8a19a 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -51,6 +51,10 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { this._fbq = null; } + _applyPurposeConsents() { + /* this implementation doesn't work with consents */ + } + /** * Gets the identifier for Facebook Pixel. * diff --git a/packages/plugin-analytic-google/src/GoogleAnalytics4.ts b/packages/plugin-analytic-google/src/GoogleAnalytics4.ts index 3d6fb1f5..716fc802 100644 --- a/packages/plugin-analytic-google/src/GoogleAnalytics4.ts +++ b/packages/plugin-analytic-google/src/GoogleAnalytics4.ts @@ -20,7 +20,7 @@ export type AnalyticGoogleSettings = { */ export class GoogleAnalytics4 extends AbstractAnalytic { #config: AnalyticGoogleSettings; - #consentSettings?: ConsentSettings; + _consentSettings?: ConsentSettings; static get $dependencies(): Dependencies { return [ @@ -60,7 +60,7 @@ export class GoogleAnalytics4 extends AbstractAnalytic { this._analyticScriptUrl = `https://www.googletagmanager.com/gtag/js?id=${this.config.service}`; - this.#consentSettings = this.config.consentSettings; + this._consentSettings = this.config.consentSettings; } /** * Hits custom event of given with given data @@ -97,7 +97,7 @@ export class GoogleAnalytics4 extends AbstractAnalytic { this._applyPurposeConsents(purposeConsents); this._ga4Script('consent', 'update', { - ...this.#consentSettings, + ...this._consentSettings, }); } @@ -109,12 +109,12 @@ export class GoogleAnalytics4 extends AbstractAnalytic { if ( purposeConsents && typeof purposeConsents === 'object' && - this.#consentSettings + this._consentSettings ) { if (purposeConsents['1']) { - this.#consentSettings.analytics_storage = 'granted'; + this._consentSettings.analytics_storage = 'granted'; } else { - this.#consentSettings.analytics_storage = 'denied'; + this._consentSettings.analytics_storage = 'denied'; } } } @@ -135,7 +135,7 @@ export class GoogleAnalytics4 extends AbstractAnalytic { this._enable = true; this._ga4Script('consent', 'default', { - ...this.#consentSettings, + ...this._consentSettings, wait_for_update: this.config.waitForUpdateTimeout, }); @@ -161,10 +161,7 @@ export class GoogleAnalytics4 extends AbstractAnalytic { * @override * @inheritdoc */ - _createGlobalDefinition() { - // _createGlobalDefinition is called only on client, therefore window is defined - const window = this._window.getWindow()!; - + _createGlobalDefinition(window: globalThis.Window) { window.dataLayer = window.dataLayer || []; this._ga4Script = function (...rest: unknown[]) { diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index 39b6547c..3b35c34c 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -4,7 +4,7 @@ import { pluginLoader } from '@ima/core'; import { GoogleAnalytics4, type AnalyticGoogleSettings, -} from './GoogleAnalytics4.js'; +} from './GoogleAnalytics4'; declare global { interface Window { diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index 64089ae0..53533f06 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -96,13 +96,9 @@ export abstract class AbstractAnalytic { * Applies Purpose Consents to respect GDPR, see https://github.com/InteractiveAdvertisingBureau/GDPR-Transparency-and-Consent-Framework * * @abstract - * @param _purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel + * @param purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel */ - _applyPurposeConsents(_purposeConsents: Record) { - throw new Error( - 'The applyPurposeConsents() method is abstract and must be overridden.' - ); - } + abstract _applyPurposeConsents(purposeConsents: Record): void; /** * Returns true if analytic is enabled. @@ -132,24 +128,15 @@ export abstract class AbstractAnalytic { * @abstract * @protected */ - _configuration() { - throw new Error( - 'The _configuration() method is abstract and must be overridden.' - ); - } + abstract _configuration(): void; /** * Creates global definition for analytics script. * * @abstract * @protected - * @param _window */ - _createGlobalDefinition(_window: globalThis.Window) { - throw new Error( - 'The _createGlobalDefinition() method is abstract and must be overridden.' - ); - } + abstract _createGlobalDefinition(window: globalThis.Window): void; /** * @protected From 36d5e85cf59fcfa4de963e73e67cc15a9b1cb9b0 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Fri, 8 Mar 2024 12:55:25 +0100 Subject: [PATCH 16/25] fix unit test --- .../src/__tests__/AbstractAnalyticSpec.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js index ce815f76..9eaaad70 100644 --- a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js +++ b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js @@ -6,6 +6,15 @@ import { AbstractAnalytic } from '../AbstractAnalytic'; import { Events as AnalyticEvents } from '../Events'; describe('AbstractAnalytic', () => { + // abstract method needs to be implemented to be able to test them or to use jest.spyOn them + class DummyAnalytic extends AbstractAnalytic { + _applyPurposeConsents() {} + hit() {} + hitPageView() {} + _configuration() {} + _createGlobalDefinition() {} + } + let abstractAnalytic = null; const _windowMock = toMockedInstance(Window, { @@ -21,11 +30,7 @@ describe('AbstractAnalytic', () => { }); beforeEach(() => { - abstractAnalytic = new AbstractAnalytic( - scriptLoader, - _windowMock, - dispatcher - ); + abstractAnalytic = new DummyAnalytic(scriptLoader, _windowMock, dispatcher); abstractAnalytic._analyticScriptName = 'dummy'; abstractAnalytic._analyticScriptUrl = 'http://example.net/script.js'; From 8d6d2ab2334a1ebb759b95200a7ff245f6e28a2f Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 11 Mar 2024 07:15:56 +0100 Subject: [PATCH 17/25] purposeConsents is optional --- packages/plugin-analytic/src/AbstractAnalytic.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index 53533f06..6d7add34 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -6,7 +6,7 @@ import { Events as AnalyticEvents } from './Events'; // @property purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel export type InitConfig = Record & { - purposeConsents: Record; + purposeConsents?: Record; }; /** @@ -62,8 +62,6 @@ export abstract class AbstractAnalytic { /** * Load analytic script, configure analytic and execute deferred hits. - * - * @returns {Promise} */ load() { if (this._window.isClient()) { From efb0c9816cf14ac959912ccd7b522a1ed4c2c731 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Mon, 11 Mar 2024 09:40:48 +0100 Subject: [PATCH 18/25] whole initConfig for init mehthod is optional --- packages/plugin-analytic/src/AbstractAnalytic.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index 6d7add34..d777086f 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -47,12 +47,12 @@ export abstract class AbstractAnalytic { * @param initConfig * @param initConfig.purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel */ - init(initConfig: InitConfig) { + init(initConfig?: InitConfig) { if (!this.isEnabled() && this._window.isClient()) { // we are on client, therefore window is defined const window = this._window.getWindow()!; - if (initConfig && initConfig.purposeConsents) { + if (initConfig?.purposeConsents) { this._applyPurposeConsents(initConfig.purposeConsents); } this._createGlobalDefinition(window); From b6231319e3dedd72c3c9345ce40d81873948bcc3 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 12 Mar 2024 15:50:56 +0100 Subject: [PATCH 19/25] CR fixes --- .../src/FacebookPixelAnalytic.ts | 32 +++++++++---------- .../plugin-analytic/src/AbstractAnalytic.ts | 2 +- .../src/__tests__/AbstractAnalyticSpec.js | 2 +- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index 39b8a19a..9d081aae 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -3,6 +3,20 @@ import { AbstractAnalytic } from '@ima/plugin-analytic'; const FB_ROOT_VARIABLE = 'fbq'; +/* + * Whole code of method _createGlobalDefinition is implementation of initialization code of Facebook Pixel from their documentation. + * Not everything used there is typed on the internet (at least I didn't find it), therefore it is typed by us here. + */ +interface FbAnalytic { + callMethod: (...params: unknown[]) => void; + queue: any[]; + push: FbAnalytic; + loaded: boolean; + version: string; +} + +type FbAnalyiticExtended = facebook.Pixel.Event & FbAnalytic; + export type AnalyticFBPixelSettings = { id: string | null; }; @@ -32,8 +46,6 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { /** * Creates a Facebook Pixel Helper instance. - * @param config - * @param {...any} rest */ constructor( config: AnalyticFBPixelSettings, @@ -176,7 +188,7 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { return false; } - let eventData: Record | null; + let eventData: Record | null; if (typeof queryOrData === 'string' && queryOrData) { eventData = { search_string: queryOrData }; @@ -218,20 +230,6 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { return; } - /* - * Whole code of this method is implementation of initialization code of Facebook Pixel from their documentation. - * Not everything used here is typed on the internet (at least I do not find it), therefore it is typed by us here. - */ - interface FbAnalytic { - callMethod: (...params: unknown[]) => void; - queue: any[]; - push: FbAnalytic; - loaded: boolean; - version: string; - } - - type FbAnalyiticExtended = facebook.Pixel.Event & FbAnalytic; - const fbAnalytic = (window[FB_ROOT_VARIABLE] = function ( ...rest: unknown[] ) { diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index d777086f..80dbbf19 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -6,7 +6,7 @@ import { Events as AnalyticEvents } from './Events'; // @property purposeConsents Purpose Consents of TCModel, see: https://www.npmjs.com/package/@iabtcf/core#tcmodel export type InitConfig = Record & { - purposeConsents?: Record; + purposeConsents?: Record; }; /** diff --git a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js index 9eaaad70..2e4c73b0 100644 --- a/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js +++ b/packages/plugin-analytic/src/__tests__/AbstractAnalyticSpec.js @@ -6,7 +6,7 @@ import { AbstractAnalytic } from '../AbstractAnalytic'; import { Events as AnalyticEvents } from '../Events'; describe('AbstractAnalytic', () => { - // abstract method needs to be implemented to be able to test them or to use jest.spyOn them + // Abstract methods must be implemented to be testable and monitored by jest.spyOn class DummyAnalytic extends AbstractAnalytic { _applyPurposeConsents() {} hit() {} From 9bebea844d3b45ffa94b805c1a1d3ade4e9a4e33 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 26 Mar 2024 14:52:52 +0100 Subject: [PATCH 20/25] Make properties and methods private if it makes sense --- .changeset/few-yaks-scream.md | 3 ++ .changeset/pink-coats-rest.md | 8 +++- .../src/FacebookPixelAnalytic.ts | 26 ++++++------- .../plugin-analytic/src/AbstractAnalytic.ts | 37 +++++++++++-------- 4 files changed, 42 insertions(+), 32 deletions(-) diff --git a/.changeset/few-yaks-scream.md b/.changeset/few-yaks-scream.md index ce7be409..5811e99e 100644 --- a/.changeset/few-yaks-scream.md +++ b/.changeset/few-yaks-scream.md @@ -10,11 +10,14 @@ Update to new version of @ima/plugin-analytic - 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. **!!** diff --git a/.changeset/pink-coats-rest.md b/.changeset/pink-coats-rest.md index 078ba0aa..04ccaf69 100644 --- a/.changeset/pink-coats-rest.md +++ b/.changeset/pink-coats-rest.md @@ -7,11 +7,15 @@ 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. @@ -66,4 +70,6 @@ Removed config from constructor of `AbstractAnalytic` // ... } } - ``` \ No newline at end of file + ``` + - Replace use of `_scriptLoader`, `_dispatcher` and `_afterLoadCallback` to `#scriptLoader`, `#dispatcher` and `#afterLoadCallback`. + Check if script is loaded by calling new method `_isLoaded()`. \ No newline at end of file diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index 9d081aae..e2744b2a 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -28,10 +28,8 @@ export type AnalyticFBPixelSettings = { */ export class FacebookPixelAnalytic extends AbstractAnalytic { #config: AnalyticFBPixelSettings; - // An identifier for Facebook Pixel. - _id: string | null; // A main function of Facebook Pixel. - _fbq: facebook.Pixel.Event | null; + #fbq: facebook.Pixel.Event | null; static get $dependencies(): Dependencies { return [ @@ -58,13 +56,11 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { this.#config = config; - this._id = null; - - this._fbq = null; + this.#fbq = null; } _applyPurposeConsents() { - /* this implementation doesn't work with consents */ + /* this implementation of FB pixel doesn't work with consents */ } /** @@ -95,7 +91,7 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { */ hit(eventName: string, eventData: Record | null = null) { try { - if (!this._fbq) { + if (!this.#fbq) { throw new Error( 'Initialize the FacebookPixelHelper instance before calling hit() method.' ); @@ -117,9 +113,9 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { } if (!eventData) { - this._fbq('track', eventName); + this.#fbq('track', eventName); } else { - this._fbq('track', eventName, eventData); + this.#fbq('track', eventName, eventData); } return true; @@ -134,7 +130,7 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { */ hitPageView(viewContentData: Record | null = null) { try { - if (!this._fbq) { + if (!this.#fbq) { throw new Error( 'Initialize the FacebookPixelHelper instance before calling hitPageView() method.' ); @@ -171,7 +167,7 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { */ hitSearch(queryOrData: Record | string | null = null) { try { - if (!this._fbq) { + if (!this.#fbq) { throw new Error( 'Initialize the FacebookPixelHelper instance before calling hitSearch() method.' ); @@ -217,8 +213,8 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { this._enable = true; - this._fbq = clientWindow[FB_ROOT_VARIABLE]; - this._fbq!('init', this.getId()); + this.#fbq = clientWindow[FB_ROOT_VARIABLE]; + this.#fbq!('init', this.getId()); } /** @@ -247,7 +243,7 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { fbAnalytic.version = '2.0'; fbAnalytic.queue = []; - this._fbq = fbAnalytic; + this.#fbq = fbAnalytic; this._configuration(); } diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index 80dbbf19..fb6f59e4 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -13,16 +13,16 @@ export type InitConfig = Record & { * Abstract analytic class */ export abstract class AbstractAnalytic { - _scriptLoader: ScriptLoaderPlugin; + #scriptLoader: ScriptLoaderPlugin; + #dispatcher: Dispatcher; + // If flag has value true then analytic script was loaded. + #loaded = false; _window: Window; - _dispatcher: Dispatcher; _analyticScriptName: string | null = null; // Analytic script url. _analyticScriptUrl: string | null = null; // If flag has value true then analytic is enabled to hit events. _enable = false; - // If flag has value true then analytic script was loaded. - _loaded = false; static get $dependencies(): Dependencies { return [ScriptLoaderPlugin, '$Window', '$Dispatcher']; @@ -33,11 +33,11 @@ export abstract class AbstractAnalytic { window: Window, dispatcher: Dispatcher ) { - this._scriptLoader = scriptLoader; + this.#scriptLoader = scriptLoader; this._window = window; - this._dispatcher = dispatcher; + this.#dispatcher = dispatcher; } /** @@ -65,20 +65,20 @@ export abstract class AbstractAnalytic { */ load() { if (this._window.isClient()) { - if (this._loaded) { + if (this._isLoaded()) { return Promise.resolve(true); } if (!this._analyticScriptUrl) { - this._afterLoadCallback(); + this.#afterLoadCallback(); return Promise.resolve(true); } - return this._scriptLoader + return this.#scriptLoader .load(this._analyticScriptUrl) .then(() => { - this._afterLoadCallback(); + this.#afterLoadCallback(); return true; }) @@ -105,6 +105,14 @@ export abstract class AbstractAnalytic { return this._enable; } + /** + * Returns true if analytic is loaded. + * @protected + */ + _isLoaded() { + return this.#loaded; + } + /** * Hit event to analytic with defined data. If analytic is not configured then * defer hit to storage. @@ -136,11 +144,8 @@ export abstract class AbstractAnalytic { */ abstract _createGlobalDefinition(window: globalThis.Window): void; - /** - * @protected - */ - _afterLoadCallback() { - this._loaded = true; + #afterLoadCallback() { + this.#loaded = true; this._configuration(); this._fireLifecycleEvent(AnalyticEvents.LOADED); } @@ -150,6 +155,6 @@ export abstract class AbstractAnalytic { * @param eventType */ _fireLifecycleEvent(eventType: AnalyticEvents) { - this._dispatcher.fire(eventType, { type: this._analyticScriptName }, true); + this.#dispatcher.fire(eventType, { type: this._analyticScriptName }, true); } } From 857edefdfb9f323f865e66069ab498960337cba2 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Thu, 28 Mar 2024 10:42:05 +0100 Subject: [PATCH 21/25] fix lint --- packages/plugin-analytic-fb-pixel/src/main.ts | 5 ++++- packages/plugin-analytic-google/src/main.ts | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/main.ts b/packages/plugin-analytic-fb-pixel/src/main.ts index b2b5447c..26cb58a8 100644 --- a/packages/plugin-analytic-fb-pixel/src/main.ts +++ b/packages/plugin-analytic-fb-pixel/src/main.ts @@ -1,6 +1,9 @@ import { pluginLoader } from '@ima/core'; -import { FacebookPixelAnalytic, type AnalyticFBPixelSettings } from './FacebookPixelAnalytic'; +import { + FacebookPixelAnalytic, + type AnalyticFBPixelSettings, +} from './FacebookPixelAnalytic'; pluginLoader.register('@ima/plugin-analytic-google', () => ({ initSettings: () => ({ diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index bf7fc285..06707efa 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -1,6 +1,9 @@ import { pluginLoader } from '@ima/core'; -import { GoogleAnalytics4, type AnalyticGoogleSettings } from './GoogleAnalytics4.js'; +import { + GoogleAnalytics4, + type AnalyticGoogleSettings, +} from './GoogleAnalytics4.js'; pluginLoader.register('@ima/plugin-analytic-google', () => ({ initSettings: () => ({ From 8fd427b57752a8bb16db3ee027ab379542d14ff3 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Thu, 28 Mar 2024 10:51:48 +0100 Subject: [PATCH 22/25] fix import --- packages/plugin-analytic-google/src/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugin-analytic-google/src/main.ts b/packages/plugin-analytic-google/src/main.ts index 06707efa..795f3efc 100644 --- a/packages/plugin-analytic-google/src/main.ts +++ b/packages/plugin-analytic-google/src/main.ts @@ -3,7 +3,7 @@ import { pluginLoader } from '@ima/core'; import { GoogleAnalytics4, type AnalyticGoogleSettings, -} from './GoogleAnalytics4.js'; +} from './GoogleAnalytics4'; pluginLoader.register('@ima/plugin-analytic-google', () => ({ initSettings: () => ({ From 0c17b3a1fc226f1d24540fcef2c33c985c0da109 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 2 Apr 2024 12:27:36 +0200 Subject: [PATCH 23/25] CR fixes --- package-lock.json | 7 ++----- packages/plugin-analytic-fb-pixel/src/types.ts | 4 ---- packages/plugin-analytic-google/package.json | 6 ++---- packages/plugin-analytic-google/src/types.ts | 4 ---- packages/plugin-analytic/src/AbstractAnalytic.ts | 4 ++-- 5 files changed, 6 insertions(+), 19 deletions(-) diff --git a/package-lock.json b/package-lock.json index e11f477a..3544c7d5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4471,8 +4471,7 @@ "node_modules/@types/gtag.js": { "version": "0.0.19", "resolved": "https://registry.npmjs.org/@types/gtag.js/-/gtag.js-0.0.19.tgz", - "integrity": "sha512-KHoDzrf9rSd0mooKN576PjExpdk/XRrNu4RQnmigsScSTSidwyOUe9kDrHz9UPKjiBrx2QEsSkexbJSgS0j72w==", - "dev": true + "integrity": "sha512-KHoDzrf9rSd0mooKN576PjExpdk/XRrNu4RQnmigsScSTSidwyOUe9kDrHz9UPKjiBrx2QEsSkexbJSgS0j72w==" }, "node_modules/@types/http-proxy": { "version": "1.17.14", @@ -21682,9 +21681,7 @@ "license": "MIT", "dependencies": { "@ima/plugin-analytic": "^5.0.2", - "@ima/plugin-script-loader": "^4.0.0" - }, - "devDependencies": { + "@ima/plugin-script-loader": "^4.0.0", "@types/gtag.js": "^0.0.19" }, "peerDependencies": { diff --git a/packages/plugin-analytic-fb-pixel/src/types.ts b/packages/plugin-analytic-fb-pixel/src/types.ts index e823e68a..26902382 100644 --- a/packages/plugin-analytic-fb-pixel/src/types.ts +++ b/packages/plugin-analytic-fb-pixel/src/types.ts @@ -24,8 +24,4 @@ declare module '@ima/core' { interface Settings { plugin: PluginSettings; } - - interface OCAliasMap { - '$Settings.plugin.analytic.fbPixel': AnalyticFBPixelSettings; - } } diff --git a/packages/plugin-analytic-google/package.json b/packages/plugin-analytic-google/package.json index 06907005..8d59ee9b 100644 --- a/packages/plugin-analytic-google/package.json +++ b/packages/plugin-analytic-google/package.json @@ -34,13 +34,11 @@ "license": "MIT", "dependencies": { "@ima/plugin-analytic": "^5.0.2", - "@ima/plugin-script-loader": "^4.0.0" + "@ima/plugin-script-loader": "^4.0.0", + "@types/gtag.js": "^0.0.19" }, "peerDependencies": { "@ima/core": ">=18.0.0", "@ima/plugin-script-loader": ">=3.1.1" - }, - "devDependencies": { - "@types/gtag.js": "^0.0.19" } } diff --git a/packages/plugin-analytic-google/src/types.ts b/packages/plugin-analytic-google/src/types.ts index 70dd6850..53b020b0 100644 --- a/packages/plugin-analytic-google/src/types.ts +++ b/packages/plugin-analytic-google/src/types.ts @@ -22,8 +22,4 @@ declare module '@ima/core' { interface Settings { plugin: PluginSettings; } - - interface OCAliasMap { - '$Settings.plugin.analytic.google4': AnalyticGoogleSettings; - } } diff --git a/packages/plugin-analytic/src/AbstractAnalytic.ts b/packages/plugin-analytic/src/AbstractAnalytic.ts index f651ac85..8cf3e8af 100644 --- a/packages/plugin-analytic/src/AbstractAnalytic.ts +++ b/packages/plugin-analytic/src/AbstractAnalytic.ts @@ -64,7 +64,7 @@ export abstract class AbstractAnalytic { */ load() { if (this._window.isClient()) { - if (this._isLoaded()) { + if (this.isLoaded()) { return Promise.resolve(true); } @@ -108,7 +108,7 @@ export abstract class AbstractAnalytic { * Returns true if analytic is loaded. * @protected */ - _isLoaded() { + isLoaded() { return this.#loaded; } From cc7c43615dfb3741692ef0e1dbb00de0e0fe54d4 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Tue, 2 Apr 2024 12:31:21 +0200 Subject: [PATCH 24/25] fix type --- packages/plugin-analytic-fb-pixel/src/types.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/types.ts b/packages/plugin-analytic-fb-pixel/src/types.ts index 26902382..e4d2f627 100644 --- a/packages/plugin-analytic-fb-pixel/src/types.ts +++ b/packages/plugin-analytic-fb-pixel/src/types.ts @@ -8,9 +8,7 @@ declare global { } export interface PluginAnalyticFBPixelSettings { - fbPixel: { - id: string | null; - }; + fbPixel: AnalyticFBPixelSettings; } declare module '@ima/core' { From cfdb4a6b4011a4b5f6d094a4a10bad44ccbc3c36 Mon Sep 17 00:00:00 2001 From: Rayus7 Date: Thu, 4 Apr 2024 11:59:13 +0200 Subject: [PATCH 25/25] FacebookPixelAnalytic: remove public getter on config --- .../src/FacebookPixelAnalytic.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts index 978b155d..358fdc97 100644 --- a/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts +++ b/packages/plugin-analytic-fb-pixel/src/FacebookPixelAnalytic.ts @@ -38,10 +38,6 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { ]; } - get config() { - return this.#config; - } - /** * Creates a Facebook Pixel Helper instance. */ @@ -69,11 +65,11 @@ export class FacebookPixelAnalytic extends AbstractAnalytic { * @returns The identifier for Facebook Pixel. */ getId() { - switch (typeof this.config.id) { + switch (typeof this.#config.id) { case 'number': - return String(this.config.id); + return String(this.#config.id); case 'string': - return this.config.id; + return this.#config.id; default: throw new TypeError( 'A Facebook Pixel identifier should be a number/string.'