feat: make externalScripts configurable via env.config.js

[arbrandes]

Description

Currently, externalScripts in initialize() cannot be configured via env.config.js, unlike other services such as loggingService, analyticsService, and authService.

This change makes externalScripts resolvable from the config object, following the same getConfig().externalScripts || externalScripts pattern already used for the other services. The loadExternalScripts call is moved after config resolution so the config-based value is available. GoogleAnalyticsLoader remains the default.

Operators who want to customize the list can do so in their env.config.js:

import { GoogleAnalyticsLoader } from '@edx/frontend-platform/scripts';
import { CustomScript } from './scripts';

const config = {
  externalScripts: [GoogleAnalyticsLoader, CustomScript],
};

export default config;

Closes #877.

LLM usage notice

Built with assistance from Claude.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.86%. Comparing base (44cc024) to head (dcecdd9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #876   +/-   ##
=======================================
  Coverage   87.85%   87.86%           
=======================================
  Files          50       50           
  Lines        1482     1483    +1     
  Branches      297      297           
=======================================
+ Hits         1302     1303    +1     
- Misses        167      169    +2     
+ Partials       13       11    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[brian-smith-tcril]

While this definitely feels like an improvement over the current state of things, I'm wondering if it makes the "happy path" for adding simple scripts too verbose.

It seems with this if someone wanted to "just add a script" they'd need to create an entire Loader class (or function that does the equivalent) for it because loadExternalScripts uses new.

frontend-platform/src/initialize.js

Lines 201 to 206 in 44cc024

	export function loadExternalScripts(externalScripts, data) {
	externalScripts.forEach(ExternalScript => {
	const script = new ExternalScript(data);
	script.loadScript();
	});
	}

This means the simplest case for adding <script src="https://example.com/foo.js"> would be

class FooLoader {
  loadScript() {
    const script = document.createElement('script');
    script.src = 'https://example.com/foo.js';
    document.head.appendChild(script);
  }
}

I would guess the site operator expectation would be that just adding "https://example.com/foo.js" to externalScripts would work.

In that case we would probably want to implement/support some sort of generic loader.

---

The other thing that stands out to me is the config: getConfig() in

frontend-platform/src/initialize.js

Lines 320 to 322 in 11e94d0

	loadExternalScripts(externalScriptsImpl, {
	config: getConfig(),
	});

I realize that was already there before this PR, and is currently used in

frontend-platform/src/scripts/GoogleAnalyticsLoader.js

Lines 6 to 8 in 44cc024

	constructor({ config }) {
	this.analyticsId = config.GOOGLE_ANALYTICS_4_ID;
	}

but it generally feels like odd coupling to me. Is the expectation that site operators will just add arbitrary things to MFE_CONFIG for their external scripts? I know we technically support that in the ConfigDocument interface:

frontend-platform/src/config.ts

Line 232 in 44cc024

[otherKey: string]: any;

but it still feels a little odd.

[mboisson]

This means the simplest case for adding <script src="https://example.com/foo.js"> would be

class FooLoader {
  loadScript() {
    const script = document.createElement('script');
    script.src = 'https://example.com/foo.js';
    document.head.appendChild(script);
  }
}

I would guess the site operator expectation would be that just adding "https://example.com/foo.js" to externalScripts would work.

In that case we would probably want to implement/support some sort of generic loader.

I definitely agree with that, although some configurability is useful. For example, to install the "CookieYes" banner, the code is:

<script id="cookieyes" type="text/javascript" src="https://cdn-cookieyes.com/client_data/REDACTED/script.js"></script>

i.e. there's the "type" and the "id" that is specified. Not sure if they are needed or only recommended practices.

[arbrandes]

@brian-smith-tcril and @mboisson

I would guess the site operator expectation would be that just adding "https://example.com/foo.js" to externalScripts would work.

This would not be flexible enough for all use cases. See Google Analytics itself.

Is the expectation that site operators will just add arbitrary things to MFE_CONFIG for their external scripts?

Yes. If you're consuming a loader somebody else published (like the Google Analytics one), it needs to be configurable, somehow.

For example, to install the "CookieYes" banner, the code is:

Precisely.

[mboisson]

There could still be a generic loader class with some level of configurability, enough to handle most cases.

[arbrandes]

@mboisson,

a generic loader class with some level of configurability, enough to handle most cases.

"Some" level of configurability would just be guesswork at this point, and no configurability beyond a URL would not be useful at all.

I believe we should offer the most general solution possible - this one, for example :) - and then once people start using it more, and if they report their use cases, we can figure out what the "minimum common denominator" is for a generic loader.

(Though at that point we'll probably be in frontend-base land entirely, so the solution will look like an app, as opposed to a loader.)

[brian-smith-tcril]

@brian-smith-tcril and @mboisson

I would guess the site operator expectation would be that just adding "https://example.com/foo.js" to externalScripts would work.

This would not be flexible enough for all use cases. See Google Analytics itself.

I 100% agree. I wasn't suggesting we only support the "just a URL" case, but instead support both the URL (via a generic loader) and custom loaders.

That being said, it seems that "just a URL" isn't really a use case. It seems most scripts people want to add need some sort of config, so supporting "just a URL" seems like something we don't really need.

---

I would like to dig into the "some level of configurability" idea a bit. A generic loader could just take an object and make a script tag with exactly those attributes on it (assuming they're valid to put on a script tag).

So for the

<script id="cookieyes" type="text/javascript" src="https://cdn-cookieyes.com/client_data/REDACTED/script.js"></script>

example the object could be

{ src: 'https://cdn-cookieyes.com/client_data/REDACTED/script.js', id: 'cookieyes', type: 'text/javascript' }

---

One main concern I have about relying on site operators making custom loaders stems from a pain point I've seen with FPF. Site operators want to use shared components in FPF without needing to publish those to NPM. Looking at the README in overhangio/tutor-mfe#290 it seems the recommended way for site operators to add scripts would require publishing a loader to NPM, so I'd expect that to be a pain point here too.

I feel like it'd be pretty great if site operators could just add

  EXTERNAL_SCRIPTS.add_items([
      ("all", "{ src: 'https://cdn-cookieyes.com/client_data/REDACTED/script.js', id: 'cookieyes', type: 'text/javascript' }"),
  ])

I'm very open to looking into other ways to mitigate that pain point. Maybe that looks like publishing some common loaders to NPM (or finding people throughout the community to publish/maintain them). Maybe that looks like publishing a generic loader to NPM and having a reasonable config shape for that loader (that isn't tied to frontend-platform or frontend-base).

Overall this PR is a huge improvement over the current "externalScripts is just hardcoded GA" state of things, but I'd like to make sure we think through the site operator experience a bit more before landing this.

[brian-smith-tcril]

Ok, after discussing this/thinking this through I agree that landing this PR as-is is the right call.

The one remaining concern I have is that this changes what is required of site operators to enable GA (moving from just setting GOOGLE_ANALYTICS_4_ID as documented here to needing to create build-time config)

I think that would require a DEPR (which I think we should make!), but for this PR just keeping it in the build by default is likely the safest option.

[arbrandes]

@brian-smith-tcril, agreed: I left GA in, so the behavior is exactly the same if people do nothing.

[mboisson]

One main concern I have about relying on site operators making custom loaders stems from a pain point I've seen with FPF. Site operators want to use shared components in FPF without needing to publish those to NPM. Looking at the README in overhangio/tutor-mfe#290 it seems the recommended way for site operators to add scripts would require publishing a loader to NPM, so I'd expect that to be a pain point here too.

Reading the above PR, it looks like the easiest way would not be to publish to NPM, but to rather implement a build time definition patch with the loader's code. A generic loader published on NPM would make it easier for all users, but once you're already configuring stuff with tutor's hooks and patches, it is not too foreign to do it with a build time patch.

[arbrandes]

@mboisson

to rather implement a build time definition patch with the loader's code

Exactly. I just added an example of this to the README. (Used Cooki👀, because why not.)

[brian-smith-tcril]