web-ext-config ES Module Support

[xM8WVqaG]

Is this a feature request or a bug?

Feature

What is the current behavior?

If you've migrated your browser extension to no longer use CommonJS modules then Node will no longer parse the web-ext-config.js file without additional changes.

This is the correct behavior as per the web-ext docs:

The file should be a CommonJS module as understood by NodeJS and must export each configuration value.

and

Automatic discovery of configuration files

web-ext will load existing configuration files in the following order:

• A config file named .web-ext-config.js in your home directory, for example: - On Windows: C:\Users\<username>\.web-ext-config.js - On macOS: /Users/<username>/.web-ext-config.js - On Linux: /home/<username>/.web-ext-config.js
• A config property named "webExt" included in a package.json file in the current directory.
• A config file named web-ext-config.js in the current directory.

https://extensionworkshop.com/documentation/develop/getting-started-with-web-ext/#setting-option-defaults-in-a-configuration-file

Example

In package.json you have set:

{
  "type": "module",
}

and you then run any web-ext command you get an error from Node.

$ npx web-ext lint
Applying config files: ./package.json, ./web-ext-config.js

UsageError: Cannot read config file: /REDACTED/web-ext-config.js
Error: require() of ES Module /REDACTED/web-ext-config.js from /REDACTED/node_modules/import-fresh/index.js not supported.
web-ext-config.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead rename web-ext-config.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /REDACTED/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

What is the expected or desired behavior?

Quickest win would be to add ./web-ext-config.cjs to the pool of default files checked.

Less quick would be to allow ES Modules export {} in web-ext-config.{,m}js.

The workaround for now is to add --config web-ext-config.cjs as a suffix to web-ext commands.

It seems setting config in the webExt block in package.json doesn't work:

{
  "webExt": {
    "config": "web-ext-config.cjs"
  }
}

Version information (for bug reports)

node --version && npm --version && npx web-ext --version
v16.15.1
8.11.0
7.4.0

[xM8WVqaG]

I had a bit of time to look at this again this afternoon.

Currently the config import functionality is provided by import-fresh which has an outstanding issue for adding support for ES Modules.

Although there is a fork with ESM support (@small-tech/import-fresh)
it's probably better to just cache bust with a dynamic import() and await, for example:

diff --git a/.babelrc b/.babelrc
index 9d545e5..54e0e73 100644
--- a/.babelrc
+++ b/.babelrc
@@ -14,7 +14,8 @@
       "include": [
         "WEBEXT_BUILD_ENV"
       ]
-    }] 
+    }],
+    ["@babel/plugin-syntax-import-assertions"]
   ],
   "env": {
     "test": {
diff --git a/package-lock.json b/package-lock.json
index bd1d245..c328720 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -49,6 +49,7 @@
         "@babel/cli": "7.20.7",
         "@babel/core": "7.20.12",
         "@babel/eslint-parser": "7.19.1",
+        "@babel/plugin-syntax-import-assertions": "7.20.0",
         "@babel/preset-env": "7.20.2",
         "@babel/preset-flow": "7.18.6",
         "@babel/register": "7.18.9",
diff --git a/package.json b/package.json
index 9e6ce55..8bbcfab 100644
--- a/package.json
+++ b/package.json
@@ -95,6 +95,7 @@
     "@babel/cli": "7.20.7",
     "@babel/core": "7.20.12",
     "@babel/eslint-parser": "7.19.1",
+    "@babel/plugin-syntax-import-assertions": "7.20.0",
     "@babel/preset-env": "7.20.2",
     "@babel/preset-flow": "7.18.6",
     "@babel/register": "7.18.9",
diff --git a/src/config.js b/src/config.js
index 1de18a2..e2bbbc9 100644
--- a/src/config.js
+++ b/src/config.js
@@ -2,7 +2,6 @@
 import os from 'os';
 import path from 'path';
 
-import importFresh from 'import-fresh';
 import camelCase from 'camelcase';
 import decamelize from 'decamelize';
 
@@ -127,7 +126,7 @@ export function applyConfigToArgv({
   return newArgv;
 }
 
-export function loadJSConfigFile(filePath: string): Object {
+export async function loadJSConfigFile(filePath: string): Object {
   const resolvedFilePath = path.resolve(filePath);
   log.debug(
     `Loading JS config file: "${filePath}" ` +
@@ -135,7 +134,9 @@ export function loadJSConfigFile(filePath: string): Object {
   );
   let configObject;
   try {
-    configObject = importFresh(resolvedFilePath);
+    configObject = resolvedFilePath.includes('json') ?
+      (await import(`${resolvedFilePath}?cacheBust=${new Date().getTime()}`, { assert: { type: 'json' } })).default :
+      (await import(`${resolvedFilePath}?cacheBust=${new Date().getTime()}`)).default;
   } catch (error) {
     log.debug('Handling error:', error);
     throw new UsageError(
diff --git a/src/program.js b/src/program.js
index 89f2f21..6da4793 100644
--- a/src/program.js
+++ b/src/program.js
@@ -336,8 +336,8 @@ export class Program {
         );
       }
 
-      configFiles.forEach((configFileName) => {
-        const configObject = loadJSConfigFile(configFileName);
+      configFiles.forEach(async (configFileName) => {
+        const configObject = await loadJSConfigFile(configFileName);
         adjustedArgv = applyConfigToArgv({
           argv: adjustedArgv,
           argvFromCLI: argv,

This (partially) works for ES Modules but Common JS continues to throw an exception
because Node needs the .cjs extension to know to use the Common JS parser.

Another simpler solution (the example below fully breaks backwards compatibility, but it could be adapted to search for both .js and .cjs) would be to search for web-ext-config.cjs and defer the more the breaking ES Module config 8.0.0:

diff --git a/src/config.js b/src/config.js
index 1de18a2..d632014 100644
--- a/src/config.js
+++ b/src/config.js
@@ -163,7 +163,7 @@ type DiscoverConfigFilesParams = {
 export async function discoverConfigFiles({
   getHomeDir = os.homedir,
 }: DiscoverConfigFilesParams = {}): Promise<Array<string>> {
-  const magicConfigName = 'web-ext-config.js';
+  const magicConfigName = 'web-ext-config.cjs';
 
   // Config files will be loaded in this order.
   const possibleConfigs = [

This doesn't unbreak the current exception raised when parsing a Common JS web-ext-config.js when Node is configured for ESM, but the error message from Node (and maybe some documentation) would at least give an easy migration.

Does anybody have any thoughts on the best way to move this forward?

The 7.0.0 patch notes say:

web-ext npm package is now a pure ESM package (#2405), and so it breaks backward compatibility for requiring web-ext from nodejs CommonJS modules.

But currently the only way to continue to use web-ext-config config files after updating to a pure ESM package is with a command line argument.