[ContentFeature] de-decouple build specific code from content-features #1545

dbajpeyi · 2025-03-05T15:14:52Z

Asana Task/Github Issue:

Description

Testing Steps

Checklist

Please tick all that apply:

I have tested this change locally
I have tested this change locally in all supported browsers
This change will be visible to users
I have added automated tests that cover this change
I have ensured the change is gated by config
This change was covered by a ship review
This change was covered by a tech design
Any dependent config has been merged

netlify · 2025-03-05T15:15:10Z

✅ Deploy Preview for content-scope-scripts ready!

Name	Link
🔨 Latest commit	`91e99dc`
🔍 Latest deploy log	https://app.netlify.com/sites/content-scope-scripts/deploys/67cad8e94515a1000813e202
😎 Deploy Preview	https://deploy-preview-1545--content-scope-scripts.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2025-03-05T15:15:37Z

Temporary Branch Update

The temporary branch has been updated with the latest changes. Below are the details:

Branch Name: pr-releases/pr-1545
Commit Hash: 5b6344d7fdfe8a3a1588868ac835ed9994deb96e
Install Command: npm i github:duckduckgo/content-scope-scripts#5b6344d7fdfe8a3a1588868ac835ed9994deb96e

Please use the above install command to update to the latest version.

github-actions · 2025-03-05T15:15:54Z

[Beta] Generated file diff

Time updated: Fri, 07 Mar 2025 11:31:42 GMT

Android

    - android/autofillPasswordImport.js

android/brokerProtection.js
android/contentScope.js

File has changed

Chrome

    - chrome/inject.js

File has changed

Chrome-mv3

    - chrome-mv3/inject.js

File has changed

Firefox

    - firefox/inject.js

File has changed

Integration

    - integration/contentScope.js

File has changed

Windows

    - windows/contentScope.js

File has changed

Apple

    - dist/contentScope.js

dist/contentScopeIsolated.js

File has changed

jonathanKingston · 2025-03-05T17:34:05Z

injected/entry-points/android.js

@@ -31,10 +30,14 @@ function initCode() {
        debug: processedConfig.debug,
    });

+    const importConfig = {


Maybe move this into content-scope-features to remove the need for it being in all files?

jonathanKingston · 2025-03-05T17:36:00Z

injected/src/features/web-compat.js

@@ -463,7 +463,7 @@ export class WebCompat extends ContentFeature {

    mediaSessionFix() {
        try {
-            if (window.navigator.mediaSession && import.meta.injectName !== 'integration') {
+            if (window.navigator.mediaSession && this.injectName !== 'integration') {


Should this be this.importConfig.injectName?

There's a getter in the base class.

jonathanKingston · 2025-03-05T17:39:52Z

injected/src/content-scope-features.js

@@ -39,7 +37,7 @@ export function load(args) {
        return;
    }

-    const featureNames = typeof import.meta.injectName === 'string' ? platformSupport[import.meta.injectName] : [];


In the for loop below I think you'll need to pass in importConfig into the ContentFeature, probably to the constructor.

…uckduckgo/content-scope-scripts into dbajpeyi/refactor/decouple-import-meta

jonathanKingston · 2025-03-06T14:29:32Z

injected/src/content-feature.js

-        this.#args = null;
+    constructor(featureName, importConfig) {
+        super(featureName);
+        this.args = null;


Suggested change

this.args = null;

this.#args = null;

jonathanKingston · 2025-03-06T14:53:43Z

injected/src/content-scope-features.js

+        const featureInstance = new ContentFeature(featureName, importConfig);
        featureInstance.callLoad(args);


Suggested change

const featureInstance = new ContentFeature(featureName, importConfig);

featureInstance.callLoad(args);

const featureInstance = new ContentFeature(featureName, importConfig, args);

featureInstance.callLoad();

Later on:

featureInstance.callInit(args);

This change would allow us to move the initLoadArgs it super constructor.

jonathanKingston · 2025-03-06T15:03:13Z

injected/entry-points/chrome.js

@@ -54,7 +54,6 @@ function init() {
          platform: {
              name: 'extension'
          },
-          trackerLookup: ${JSON.stringify(trackerLookup)},
          site: ${JSON.stringify(siteObject)},
          documentOriginIsTracker: ${documentOriginIsTracker},


Suggested change

documentOriginIsTracker: ${documentOriginIsTracker},

jonathanKingston · 2025-03-06T15:04:29Z

injected/src/content-feature.js

+    constructor(featureName, importConfig) {
+        super(featureName);


Suggested change

constructor(featureName, importConfig) {

super(featureName);

constructor(featureName, importConfig, args) {

super(featureName, args);

jonathanKingston · 2025-03-06T15:05:14Z

injected/src/config-feature.js

+    initLoadArgs(loadArgs) {
+        const { bundledConfig, site, platform } = loadArgs;
+        this.#bundledConfig = bundledConfig;
+        this.#args = loadArgs;
+        // If we have a bundled config, treat it as a regular config
+        // This will be overriden by the remote config if it is available
+        if (this.#bundledConfig && this.#args) {
+            const enabledFeatures = computeEnabledFeatures(bundledConfig, site.domain, platform.version);
+            this.#args.featureSettings = parseFeatureSettings(bundledConfig, enabledFeatures);
+        }
+    }


All this moves into the constructor.

jonathanKingston · 2025-03-06T15:05:24Z

injected/src/config-feature.js

+    /**
+     * @param {any} name
+     */
+    constructor(name) {


Suggested change

constructor(name) {

constructor(name, args) {

jonathanKingston · 2025-03-06T15:07:43Z

injected/src/content-feature.js

    init(args) {}

    callInit(args) {
        const mark = this.monitor.mark(this.name + 'CallInit');
-        this.#args = args;
+        this.args = args;


This becomes a method called setArgs. That we call from the constructor and callInit. We remove these two lines from callLoad.

jonathanKingston · 2025-03-06T15:08:43Z

injected/src/content-feature.js

-        }
-        this.#trackerLookup = args.trackerLookup;
-        this.#documentOriginIsTracker = args.documentOriginIsTracker;
+        this.initLoadArgs(args);


This initLoadArgs becomes part of the constructor (just calling super with the args).

jonathanKingston · 2025-03-06T15:09:34Z

injected/src/content-feature.js

-        }
-        this.#trackerLookup = args.trackerLookup;
-        this.#documentOriginIsTracker = args.documentOriginIsTracker;
+        this.initLoadArgs(args);
        this.load(args);


Suggested change

this.load(args);

this.load(this.#args);

(this passing in is legacy btw, I think there's very few uses of it).

(wip) refactor: move utility function to a separate class

6f3d8d1

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch from f87255a to c5e86a4 Compare March 5, 2025 15:28

Merge branch 'main' into dbajpeyi/refactor/decouple-import-meta

a4840a2

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch 4 times, most recently from 894cce5 to 512b303 Compare March 5, 2025 16:32

refactor: parameterize trackerLookup

e7e6721

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch from 512b303 to e7e6721 Compare March 5, 2025 17:26

jonathanKingston reviewed Mar 5, 2025

View reviewed changes

dbajpeyi changed the title ~~(wip) refactor: move utility function to a separate class~~ (wip) refactor: move utility function to a config class Mar 5, 2025

jonathanKingston reviewed Mar 5, 2025

View reviewed changes

dbajpeyi added 3 commits March 6, 2025 11:05

Merge branch 'main' into dbajpeyi/refactor/decouple-import-meta

7c81cb2

refactor: move out import config into content-scope-features

998e9cc

Merge branch 'dbajpeyi/refactor/decouple-import-meta' of github.com:d…

6476ae4

…uckduckgo/content-scope-scripts into dbajpeyi/refactor/decouple-import-meta

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch from 917f4f8 to db3cdf8 Compare March 6, 2025 14:04

dbajpeyi changed the title ~~(wip) refactor: move utility function to a config class~~ [ContentFeature] de-decouple build specific code from logic Mar 6, 2025

jonathanKingston reviewed Mar 6, 2025

View reviewed changes

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch 2 times, most recently from d309b19 to 85dfc98 Compare March 6, 2025 14:41

jonathanKingston reviewed Mar 6, 2025

View reviewed changes

fix: add content feature type

363da49

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch from 85dfc98 to 363da49 Compare March 6, 2025 15:03

jonathanKingston reviewed Mar 6, 2025

View reviewed changes

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch 3 times, most recently from 3ac99d4 to 3239109 Compare March 7, 2025 10:33

test: fix unit test config passing

2562c00

dbajpeyi force-pushed the dbajpeyi/refactor/decouple-import-meta branch from 83024d3 to 2562c00 Compare March 7, 2025 10:44

dbajpeyi changed the title ~~[ContentFeature] de-decouple build specific code from logic~~ [ContentFeature] de-decouple build specific code from content-features Mar 7, 2025

Merge branch 'main' into dbajpeyi/refactor/decouple-import-meta

91e99dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ContentFeature] de-decouple build specific code from content-features #1545

[ContentFeature] de-decouple build specific code from content-features #1545

dbajpeyi commented Mar 5, 2025 •

edited

Loading

netlify bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

jonathanKingston Mar 5, 2025

jonathanKingston Mar 5, 2025

dbajpeyi Mar 5, 2025

jonathanKingston Mar 5, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

jonathanKingston Mar 6, 2025

		const featureInstance = new ContentFeature(featureName, importConfig);
		featureInstance.callLoad(args);

[ContentFeature] de-decouple build specific code from content-features #1545

Are you sure you want to change the base?

[ContentFeature] de-decouple build specific code from content-features #1545

Conversation

dbajpeyi commented Mar 5, 2025 • edited Loading

Description

Testing Steps

Checklist

netlify bot commented Mar 5, 2025 • edited Loading

✅ Deploy Preview for content-scope-scripts ready!

github-actions bot commented Mar 5, 2025 • edited Loading

Temporary Branch Update

github-actions bot commented Mar 5, 2025 • edited Loading

[Beta] Generated file diff

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbajpeyi commented Mar 5, 2025 •

edited

Loading

netlify bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading