electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.73k stars 1.74k forks source link

ESM Config filetype causes import issues in external dep (pkg read-config-file) #8389

Closed jbool24 closed 2 months ago

jbool24 commented 4 months ago

The current external dependency used to load the config file is not prepared to handle ESM and therefore breaks the build if using native ESM modules and an external config (ESM js file).

util/config.ts calls to --> read-config-file-main.ts#L19 to load the config file which causes an error as there are no checks if we passed an ESM module type *.js file and does not use dynamic imports to load the file.

How to reproduce.

  1. Create a vanilla project with an external config and electron-builder installed
  2. add type: "module" to package.json
  3. Run yarn run electron-builder build -c ./PATH_TO_ESM_JS_FILE to start build

The following error is presented:

❯ yarn run electron-builder build -c ./electron-builder.config.js                                                                                                                                                                  ✗ staging 
yarn run v1.22.21
$ /home/{{PATH_REDACTED}}/node_modules/.bin/electron-builder build -c ./electron-builder.config.js
  • electron-builder  version=24.13.3 os=6.5.0-44-generic
  ⨯ require() of ES Module /home/{{PATH_REDACTED}}/electron-builder.config.js from /home/{{PATH_REDACTED}}/node_modules/read-config-file/out/main.js not supported.
electron-builder.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 either rename electron-builder.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 /home/{{PATH_REDACTED}}/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).
  failedTask=build stackTrace=Error [ERR_REQUIRE_ESM]: require() of ES Module /home/{{PATH_REDACTED}}/electron-builder.config.js from /home/{{PATH_REDACTED}}p/node_modules/read-config-file/out/main.js not supported.
electron-builder.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 either rename electron-builder.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 /home/{{PATH_REDACTED}}/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).
mmaietta commented 4 months ago

I'm not sure what you'd like us to do here for electron-builder since this is innately a limitation/issue of an upstream dependency, read-config-file. Please open an issue there if interested in having ESM js supported.

jbool24 commented 3 months ago

@mmaietta Hi Mike! I am documenting this here so that I can link it to a ticket for the upstream lib (I'm going to create one in read-config-file). I've found a bunch of people using electron-builder and slamming heads into tables because the docs made it seem like it should load an ESM based config file. But the required lib (read-config-file specifically) is not ESM friendly currently.

mmaietta commented 3 months ago

I'm wondering if it may just be worthwhile to copy the code and internalizing it within electron-builder then?

jbool24 commented 3 months ago

@mmaietta That would allow this project to control that logic. What would the trade-off be if it was copied and maintained within here?

mmaietta commented 3 months ago

Purely makes more maintenance for me 😅

I'll see if I can easily migrate/optimize it over as I see multiple functions already existing in electron-builder's codebase.

jbool24 commented 3 months ago

@mmaietta Maybe, hold off and leave that to each user case to monkey patch locally. This library might be updated soon, if not by them I'll submit a PR. No need for increasing workload :wink:

I originally was going to use the builder API directly to get around the loading but got things to work except kinda weirdly with different file types and explicit file extension naming. For now I'd say pause until it blocks more people proving in needs to change.

mmaietta commented 2 months ago

Looks like we may need to internalize the logic within electron-builder after all. There's other issues appearing with the updated package. Ref: https://github.com/electron-userland/electron-builder/issues/8451 https://github.com/electron-userland/electron-builder/issues/8435

jbool24 commented 2 months ago

@mmaietta :ok_hand: yes looking at those issues I agree it probably makes sense now.

mmaietta commented 2 months ago

@jbool24 can you please test this patch? I've added functionality for handling module type to the config parsing.

app-builder-lib+25.0.5.patch

diff --git a/node_modules/app-builder-lib/out/codeSign/windowsCodeSign.js b/node_modules/app-builder-lib/out/codeSign/windowsCodeSign.js
index b470adf..7ad9ca8 100644
--- a/node_modules/app-builder-lib/out/codeSign/windowsCodeSign.js
+++ b/node_modules/app-builder-lib/out/codeSign/windowsCodeSign.js
@@ -13,7 +13,7 @@ const bundledTool_1 = require("../util/bundledTool");
 const fs_extra_1 = require("fs-extra");
 const os = require("os");
 const path = require("path");
-const platformPackager_1 = require("../platformPackager");
+const resolve_1 = require("../util/resolve");
 const flags_1 = require("../util/flags");
 const vm_1 = require("../vm/vm");
 function getSignVendorPath() {
@@ -34,7 +34,7 @@ async function sign(options, packager) {
     else {
         hashes = Array.isArray(hashes) ? hashes : [hashes];
     }
-    const executor = (await (0, platformPackager_1.resolveFunction)(packager.appInfo.type, options.options.sign, "sign")) || doSign;
+    const executor = (await (0, resolve_1.resolveFunction)(packager.appInfo.type, options.options.sign, "sign")) || doSign;
     let isNest = false;
     for (const hash of hashes) {
         const taskConfiguration = { ...options, hash, isNest };
diff --git a/node_modules/app-builder-lib/out/config/config.js b/node_modules/app-builder-lib/out/config/config.js
new file mode 100644
index 0000000..c5eb329
--- /dev/null
+++ b/node_modules/app-builder-lib/out/config/config.js
@@ -0,0 +1,257 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.getConfig = getConfig;
+exports.doMergeConfigs = doMergeConfigs;
+exports.validateConfiguration = validateConfiguration;
+exports.computeDefaultAppDirectory = computeDefaultAppDirectory;
+const builder_util_1 = require("builder-util");
+const fs_extra_1 = require("fs-extra");
+const lazy_val_1 = require("lazy-val");
+const path = require("path");
+const loadConfig_1 = require("./loadConfig");
+const rectCra_1 = require("../presets/rectCra");
+const version_1 = require("../version");
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const validateSchema = require("@develar/schema-utils");
+// https://github.com/electron-userland/electron-builder/issues/1847
+function mergePublish(config, configFromOptions) {
+    // if config from disk doesn't have publish (or object), no need to handle, it will be simply merged by deepAssign
+    const publish = Array.isArray(config.publish) ? configFromOptions.publish : null;
+    if (publish != null) {
+        delete configFromOptions.publish;
+    }
+    (0, builder_util_1.deepAssign)(config, configFromOptions);
+    if (publish == null) {
+        return;
+    }
+    const listOnDisk = config.publish;
+    if (listOnDisk.length === 0) {
+        config.publish = publish;
+    }
+    else {
+        // apply to first
+        Object.assign(listOnDisk[0], publish);
+    }
+}
+async function getConfig(projectDir, configPath, configFromOptions, packageMetadata = new lazy_val_1.Lazy(() => (0, loadConfig_1.orNullIfFileNotExist)((0, fs_extra_1.readJson)(path.join(projectDir, "package.json"))))) {
+    const configRequest = { packageKey: "build", configFilename: "electron-builder", projectDir, packageMetadata };
+    const configAndEffectiveFile = await (0, loadConfig_1.getConfig)(configRequest, configPath);
+    const config = configAndEffectiveFile == null ? {} : configAndEffectiveFile.result;
+    if (configFromOptions != null) {
+        mergePublish(config, configFromOptions);
+    }
+    if (configAndEffectiveFile != null) {
+        builder_util_1.log.info({ file: configAndEffectiveFile.configFile == null ? 'package.json ("build" field)' : configAndEffectiveFile.configFile }, "loaded configuration");
+    }
+    if (config.extends == null && config.extends !== null) {
+        const metadata = (await packageMetadata.value) || {};
+        const devDependencies = metadata.devDependencies;
+        const dependencies = metadata.dependencies;
+        if ((dependencies != null && "react-scripts" in dependencies) || (devDependencies != null && "react-scripts" in devDependencies)) {
+            config.extends = "react-cra";
+        }
+        else if (devDependencies != null && "electron-webpack" in devDependencies) {
+            let file = "electron-webpack/out/electron-builder.js";
+            try {
+                file = require.resolve(file);
+            }
+            catch (ignore) {
+                file = require.resolve("electron-webpack/electron-builder.yml");
+            }
+            config.extends = `file:${file}`;
+        }
+    }
+    const parentConfigs = await loadParentConfigsRecursively(config.extends, async (configExtend) => {
+        if (configExtend === "react-cra") {
+            const result = await (0, rectCra_1.reactCra)(projectDir);
+            builder_util_1.log.info({ preset: configExtend }, "loaded parent configuration");
+            return result;
+        }
+        else {
+            const { configFile, result } = await (0, loadConfig_1.loadParentConfig)(configRequest, configExtend);
+            builder_util_1.log.info({ file: configFile }, "loaded parent configuration");
+            return result;
+        }
+    });
+    return doMergeConfigs([...parentConfigs, config]);
+}
+function asArray(value) {
+    return Array.isArray(value) ? value : typeof value === "string" ? [value] : [];
+}
+async function loadParentConfigsRecursively(configExtends, loader) {
+    const configs = [];
+    for (const configExtend of asArray(configExtends)) {
+        const result = await loader(configExtend);
+        const parentConfigs = await loadParentConfigsRecursively(result.extends, loader);
+        configs.push(...parentConfigs, result);
+    }
+    return configs;
+}
+// normalize for easy merge
+function normalizeFiles(configuration, name) {
+    let value = configuration[name];
+    if (value == null) {
+        return;
+    }
+    if (!Array.isArray(value)) {
+        value = [value];
+    }
+    itemLoop: for (let i = 0; i < value.length; i++) {
+        let item = value[i];
+        if (typeof item === "string") {
+            // merge with previous if possible
+            if (i !== 0) {
+                let prevItemIndex = i - 1;
+                let prevItem;
+                do {
+                    prevItem = value[prevItemIndex--];
+                } while (prevItem == null);
+                if (prevItem.from == null && prevItem.to == null) {
+                    if (prevItem.filter == null) {
+                        prevItem.filter = [item];
+                    }
+                    else {
+                        ;
+                        prevItem.filter.push(item);
+                    }
+                    value[i] = null;
+                    continue itemLoop;
+                }
+            }
+            item = {
+                filter: [item],
+            };
+            value[i] = item;
+        }
+        else if (Array.isArray(item)) {
+            throw new Error(`${name} configuration is invalid, nested array not expected for index ${i}: ${item}`);
+        }
+        // make sure that merge logic is not complex - unify different presentations
+        if (item.from === ".") {
+            item.from = undefined;
+        }
+        if (item.to === ".") {
+            item.to = undefined;
+        }
+        if (item.filter != null && typeof item.filter === "string") {
+            item.filter = [item.filter];
+        }
+    }
+    configuration[name] = value.filter(it => it != null);
+}
+function isSimilarFileSet(value, other) {
+    return value.from === other.from && value.to === other.to;
+}
+function mergeFilters(value, other) {
+    return asArray(value).concat(asArray(other));
+}
+function mergeFileSets(lists) {
+    const result = [];
+    for (const list of lists) {
+        for (const item of list) {
+            const existingItem = result.find(i => isSimilarFileSet(i, item));
+            if (existingItem) {
+                existingItem.filter = mergeFilters(item.filter, existingItem.filter);
+            }
+            else {
+                result.push(item);
+            }
+        }
+    }
+    return result;
+}
+/**
+ * `doMergeConfigs` takes configs in the order you would pass them to
+ * Object.assign as sources.
+ */
+function doMergeConfigs(configs) {
+    for (const config of configs) {
+        normalizeFiles(config, "files");
+        normalizeFiles(config, "extraFiles");
+        normalizeFiles(config, "extraResources");
+    }
+    const result = (0, builder_util_1.deepAssign)(getDefaultConfig(), ...configs);
+    // `deepAssign` prioritises latter configs, while `mergeFilesSets` prioritises
+    // former configs, so we have to reverse the order, because latter configs
+    // must have higher priority.
+    configs = configs.slice().reverse();
+    result.files = mergeFileSets(configs.map(config => { var _a; return ((_a = config.files) !== null && _a !== void 0 ? _a : []); }));
+    return result;
+}
+function getDefaultConfig() {
+    return {
+        directories: {
+            output: "dist",
+            buildResources: "build",
+        },
+    };
+}
+const schemeDataPromise = new lazy_val_1.Lazy(() => (0, fs_extra_1.readJson)(path.join(__dirname, "..", "..", "scheme.json")));
+async function validateConfiguration(config, debugLogger) {
+    const extraMetadata = config.extraMetadata;
+    if (extraMetadata != null) {
+        if (extraMetadata.build != null) {
+            throw new builder_util_1.InvalidConfigurationError(`--em.build is deprecated, please specify as -c"`);
+        }
+        if (extraMetadata.directories != null) {
+            throw new builder_util_1.InvalidConfigurationError(`--em.directories is deprecated, please specify as -c.directories"`);
+        }
+    }
+    const oldConfig = config;
+    if (oldConfig.npmSkipBuildFromSource === false) {
+        throw new builder_util_1.InvalidConfigurationError(`npmSkipBuildFromSource is deprecated, please use buildDependenciesFromSource"`);
+    }
+    if (oldConfig.appImage != null && oldConfig.appImage.systemIntegration != null) {
+        throw new builder_util_1.InvalidConfigurationError(`appImage.systemIntegration is deprecated, https://github.com/TheAssassin/AppImageLauncher is used for desktop integration"`);
+    }
+    // noinspection JSUnusedGlobalSymbols
+    validateSchema(await schemeDataPromise.value, config, {
+        name: `electron-builder ${version_1.PACKAGE_VERSION}`,
+        postFormatter: (formattedError, error) => {
+            if (debugLogger.isEnabled) {
+                debugLogger.add("invalidConfig", (0, builder_util_1.safeStringifyJson)(error));
+            }
+            const site = "https://www.electron.build";
+            let url = `${site}/configuration/configuration`;
+            const targets = new Set(["mac", "dmg", "pkg", "mas", "win", "nsis", "appx", "linux", "appimage", "snap"]);
+            const dataPath = error.dataPath == null ? null : error.dataPath;
+            const targetPath = dataPath.startsWith(".") ? dataPath.substr(1).toLowerCase() : null;
+            if (targetPath != null && targets.has(targetPath)) {
+                url = `${site}/configuration/${targetPath}`;
+            }
+            return `${formattedError}\n  How to fix:
+  1. Open ${url}
+  2. Search the option name on the page (or type in into Search to find across the docs).
+    * Not found? The option was deprecated or not exists (check spelling).
+    * Found? Check that the option in the appropriate place. e.g. "title" only in the "dmg", not in the root.
+`;
+        },
+    });
+}
+const DEFAULT_APP_DIR_NAMES = ["app", "www"];
+async function computeDefaultAppDirectory(projectDir, userAppDir) {
+    if (userAppDir != null) {
+        const absolutePath = path.resolve(projectDir, userAppDir);
+        const stat = await (0, builder_util_1.statOrNull)(absolutePath);
+        if (stat == null) {
+            throw new builder_util_1.InvalidConfigurationError(`Application directory ${userAppDir} doesn't exist`);
+        }
+        else if (!stat.isDirectory()) {
+            throw new builder_util_1.InvalidConfigurationError(`Application directory ${userAppDir} is not a directory`);
+        }
+        else if (projectDir === absolutePath) {
+            builder_util_1.log.warn({ appDirectory: userAppDir }, `Specified application directory equals to project dir — superfluous or wrong configuration`);
+        }
+        return absolutePath;
+    }
+    for (const dir of DEFAULT_APP_DIR_NAMES) {
+        const absolutePath = path.join(projectDir, dir);
+        const packageJson = path.join(absolutePath, "package.json");
+        const stat = await (0, builder_util_1.statOrNull)(packageJson);
+        if (stat != null && stat.isFile()) {
+            return absolutePath;
+        }
+    }
+    return projectDir;
+}
+//# sourceMappingURL=config.js.map
\ No newline at end of file
diff --git a/node_modules/app-builder-lib/out/config/loadConfig.js b/node_modules/app-builder-lib/out/config/loadConfig.js
new file mode 100644
index 0000000..c3161c5
--- /dev/null
+++ b/node_modules/app-builder-lib/out/config/loadConfig.js
@@ -0,0 +1,126 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.findAndReadConfig = findAndReadConfig;
+exports.orNullIfFileNotExist = orNullIfFileNotExist;
+exports.orIfFileNotExist = orIfFileNotExist;
+exports.loadConfig = loadConfig;
+exports.getConfig = getConfig;
+exports.loadParentConfig = loadParentConfig;
+exports.loadEnv = loadEnv;
+const fs_1 = require("fs");
+const js_yaml_1 = require("js-yaml");
+const path = require("path");
+const dotenv_1 = require("dotenv");
+const config_file_ts_1 = require("config-file-ts");
+const dotenv_expand_1 = require("dotenv-expand");
+const resolve_1 = require("../util/resolve");
+async function readConfig(configFile, request) {
+    const data = await fs_1.promises.readFile(configFile, "utf8");
+    let result;
+    if (configFile.endsWith(".json5") || configFile.endsWith(".json")) {
+        result = require("json5").parse(data);
+    }
+    else if (configFile.endsWith(".js") || configFile.endsWith(".cjs" || configFile.endsWith(".mjs"))) {
+        const json = await orNullIfFileNotExist(fs_1.promises.readFile(path.join(process.cwd(), "package.json"), "utf8"));
+        const moduleType = json === null ? null : JSON.parse(json).type;
+        result = await (0, resolve_1.resolveModule)(moduleType, configFile);
+        if (result.default != null) {
+            result = result.default;
+        }
+        if (typeof result === "function") {
+            result = result(request);
+        }
+        result = await Promise.resolve(result);
+    }
+    else if (configFile.endsWith(".ts")) {
+        result = (0, config_file_ts_1.loadTsConfig)(configFile);
+        if (typeof result === "function") {
+            result = result(request);
+        }
+        result = await Promise.resolve(result);
+    }
+    else if (configFile.endsWith(".toml")) {
+        result = require("toml").parse(data);
+    }
+    else {
+        result = (0, js_yaml_1.load)(data);
+    }
+    return { result, configFile };
+}
+async function findAndReadConfig(request) {
+    const prefix = request.configFilename;
+    for (const configFile of [`${prefix}.yml`, `${prefix}.yaml`, `${prefix}.json`, `${prefix}.json5`, `${prefix}.toml`, `${prefix}.js`, `${prefix}.cjs`, `${prefix}.ts`]) {
+        const data = await orNullIfFileNotExist(readConfig(path.join(request.projectDir, configFile), request));
+        if (data != null) {
+            return data;
+        }
+    }
+    return null;
+}
+function orNullIfFileNotExist(promise) {
+    return orIfFileNotExist(promise, null);
+}
+function orIfFileNotExist(promise, fallbackValue) {
+    return promise.catch(e => {
+        if (e.code === "ENOENT" || e.code === "ENOTDIR") {
+            return fallbackValue;
+        }
+        throw e;
+    });
+}
+async function loadConfig(request) {
+    let packageMetadata = request.packageMetadata == null ? null : await request.packageMetadata.value;
+    if (packageMetadata == null) {
+        const json = await orNullIfFileNotExist(fs_1.promises.readFile(path.join(request.projectDir, "package.json"), "utf8"));
+        packageMetadata = json == null ? null : JSON.parse(json);
+    }
+    const data = packageMetadata == null ? null : packageMetadata[request.packageKey];
+    return data == null ? findAndReadConfig(request) : { result: data, configFile: null };
+}
+function getConfig(request, configPath) {
+    if (configPath == null) {
+        return loadConfig(request);
+    }
+    else {
+        return readConfig(path.resolve(request.projectDir, configPath), request);
+    }
+}
+async function loadParentConfig(request, spec) {
+    let isFileSpec;
+    if (spec.startsWith("file:")) {
+        spec = spec.substring("file:".length);
+        isFileSpec = true;
+    }
+    let parentConfig = await orNullIfFileNotExist(readConfig(path.resolve(request.projectDir, spec), request));
+    if (parentConfig == null && isFileSpec !== true) {
+        let resolved = null;
+        try {
+            resolved = require.resolve(spec);
+        }
+        catch (e) {
+            // ignore
+        }
+        if (resolved != null) {
+            parentConfig = await readConfig(resolved, request);
+        }
+    }
+    if (parentConfig == null) {
+        throw new Error(`Cannot find parent config file: ${spec}`);
+    }
+    return parentConfig;
+}
+async function loadEnv(envFile) {
+    const data = await orNullIfFileNotExist(fs_1.promises.readFile(envFile, "utf8"));
+    if (data == null) {
+        return null;
+    }
+    const parsed = (0, dotenv_1.parse)(data);
+    Object.entries(parsed).forEach(([key, value]) => {
+        if (!Object.prototype.hasOwnProperty.call(process.env, key)) {
+            process.env[key] = value;
+        }
+    });
+    (0, dotenv_expand_1.expand)({ parsed });
+    return parsed;
+}
+//# sourceMappingURL=loadConfig.js.map
\ No newline at end of file
diff --git a/node_modules/app-builder-lib/out/config/resolve.js b/node_modules/app-builder-lib/out/config/resolve.js
new file mode 100644
index 0000000..8965d2c
--- /dev/null
+++ b/node_modules/app-builder-lib/out/config/resolve.js
@@ -0,0 +1,54 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.resolveModule = resolveModule;
+exports.resolveFunction = resolveFunction;
+const log_1 = require("builder-util/out/log");
+const debug_1 = require("debug");
+const path = require("path");
+const url_1 = require("url");
+async function resolveModule(type, name) {
+    var _a, _b, _c;
+    const extension = path.extname(name).toLowerCase();
+    const isModuleType = type === "module";
+    try {
+        if (extension === ".mjs" || (extension === ".js" && isModuleType)) {
+            const fileUrl = (0, url_1.pathToFileURL)(name).href;
+            return await eval("import('" + fileUrl + "')");
+        }
+    }
+    catch (error) {
+        log_1.log.debug({ moduleName: name, message: (_a = error.message) !== null && _a !== void 0 ? _a : error.stack }, "Unable to dynamically import, falling back to `require`");
+    }
+    try {
+        return require(name);
+    }
+    catch (error) {
+        log_1.log.error({ moduleName: name, message: (_b = error.message) !== null && _b !== void 0 ? _b : error.stack }, "Unable to `require`");
+        throw new Error((_c = error.message) !== null && _c !== void 0 ? _c : error.stack);
+    }
+}
+async function resolveFunction(type, executor, name) {
+    if (executor == null || typeof executor !== "string") {
+        return executor;
+    }
+    let p = executor;
+    if (p.startsWith(".")) {
+        p = path.resolve(p);
+    }
+    try {
+        p = require.resolve(p);
+    }
+    catch (e) {
+        (0, debug_1.default)(e);
+        p = path.resolve(p);
+    }
+    const m = await resolveModule(type, p);
+    const namedExport = m[name];
+    if (namedExport == null) {
+        return m.default || m;
+    }
+    else {
+        return namedExport;
+    }
+}
+//# sourceMappingURL=resolve.js.map
\ No newline at end of file
diff --git a/node_modules/app-builder-lib/out/electron/electronVersion.js b/node_modules/app-builder-lib/out/electron/electronVersion.js
index 44aa50f..1f7825f 100644
--- a/node_modules/app-builder-lib/out/electron/electronVersion.js
+++ b/node_modules/app-builder-lib/out/electron/electronVersion.js
@@ -10,9 +10,9 @@ const builder_util_runtime_1 = require("builder-util-runtime");
 const builder_util_2 = require("builder-util");
 const fs_extra_1 = require("fs-extra");
 const path = require("path");
-const read_config_file_1 = require("read-config-file");
+const loadConfig_1 = require("../config/loadConfig");
 const semver = require("semver");
-const config_1 = require("../util/config");
+const config_1 = require("../config/config");
 const electronPackages = ["electron", "electron-prebuilt", "electron-prebuilt-compile", "electron-nightly"];
 async function getElectronVersion(projectDir, config) {
     if (config == null) {
@@ -58,7 +58,7 @@ async function computeElectronVersion(projectDir) {
     const potentialRootDirs = [projectDir, await (0, search_module_1.getProjectRootPath)(projectDir)];
     let dependency = null;
     for await (const dir of potentialRootDirs) {
-        const metadata = await (0, read_config_file_1.orNullIfFileNotExist)((0, fs_extra_1.readJson)(path.join(dir, "package.json")));
+        const metadata = await (0, loadConfig_1.orNullIfFileNotExist)((0, fs_extra_1.readJson)(path.join(dir, "package.json")));
         dependency = metadata ? findFromPackageMetadata(metadata) : null;
         if (dependency) {
             break;
diff --git a/node_modules/app-builder-lib/out/index.js b/node_modules/app-builder-lib/out/index.js
index 35ea025..415a399 100644
--- a/node_modules/app-builder-lib/out/index.js
+++ b/node_modules/app-builder-lib/out/index.js
@@ -6,7 +6,7 @@ exports.build = build;
 const builder_util_1 = require("builder-util");
 const builder_util_runtime_1 = require("builder-util-runtime");
 const packager_1 = require("./packager");
-const platformPackager_1 = require("./platformPackager");
+const resolve_1 = require("./util/resolve");
 const PublishManager_1 = require("./publish/PublishManager");
 var packager_2 = require("./packager");
 Object.defineProperty(exports, "Packager", { enumerable: true, get: function () { return packager_2.Packager; } });
@@ -25,8 +25,8 @@ var builder_util_runtime_2 = require("builder-util-runtime");
 Object.defineProperty(exports, "CancellationToken", { enumerable: true, get: function () { return builder_util_runtime_2.CancellationToken; } });
 var PublishManager_2 = require("./publish/PublishManager");
 Object.defineProperty(exports, "PublishManager", { enumerable: true, get: function () { return PublishManager_2.PublishManager; } });
-var platformPackager_2 = require("./platformPackager");
-Object.defineProperty(exports, "PlatformPackager", { enumerable: true, get: function () { return platformPackager_2.PlatformPackager; } });
+var platformPackager_1 = require("./platformPackager");
+Object.defineProperty(exports, "PlatformPackager", { enumerable: true, get: function () { return platformPackager_1.PlatformPackager; } });
 var forge_maker_1 = require("./forge-maker");
 Object.defineProperty(exports, "buildForge", { enumerable: true, get: function () { return forge_maker_1.buildForge; } });
 var linuxPackager_1 = require("./linuxPackager");
@@ -53,7 +53,7 @@ function build(options, packager = new packager_1.Packager(options)) {
     };
     process.once("SIGINT", sigIntHandler);
     const promise = packager.build().then(async (buildResult) => {
-        const afterAllArtifactBuild = await (0, platformPackager_1.resolveFunction)(packager.appInfo.type, buildResult.configuration.afterAllArtifactBuild, "afterAllArtifactBuild");
+        const afterAllArtifactBuild = await (0, resolve_1.resolveFunction)(packager.appInfo.type, buildResult.configuration.afterAllArtifactBuild, "afterAllArtifactBuild");
         if (afterAllArtifactBuild != null) {
             const newArtifacts = (0, builder_util_runtime_1.asArray)(await Promise.resolve(afterAllArtifactBuild(buildResult)));
             if (newArtifacts.length === 0 || !publishManager.isPublish) {
diff --git a/node_modules/app-builder-lib/out/macPackager.js b/node_modules/app-builder-lib/out/macPackager.js
index 6cc6602..16665ae 100644
--- a/node_modules/app-builder-lib/out/macPackager.js
+++ b/node_modules/app-builder-lib/out/macPackager.js
@@ -18,6 +18,7 @@ const pathManager_1 = require("./util/pathManager");
 const fs = require("fs/promises");
 const notarize_1 = require("@electron/notarize");
 const builder_util_runtime_1 = require("builder-util-runtime");
+const resolve_1 = require("./util/resolve");
 class MacPackager extends platformPackager_1.PlatformPackager {
     constructor(info) {
         super(info, core_1.Platform.MAC);
@@ -342,7 +343,7 @@ class MacPackager extends platformPackager_1.PlatformPackager {
     }
     //noinspection JSMethodCanBeStatic
     async doSign(opts, customSignOptions) {
-        const customSign = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, customSignOptions.sign, "sign");
+        const customSign = await (0, resolve_1.resolveFunction)(this.appInfo.type, customSignOptions.sign, "sign");
         const { app, platform, type, identity, provisioningProfile } = opts;
         builder_util_1.log.info({
             file: builder_util_1.log.filePath(app),
diff --git a/node_modules/app-builder-lib/out/packager.js b/node_modules/app-builder-lib/out/packager.js
index 39e4d39..96b9a5c 100644
--- a/node_modules/app-builder-lib/out/packager.js
+++ b/node_modules/app-builder-lib/out/packager.js
@@ -13,10 +13,9 @@ const asar_1 = require("./asar/asar");
 const core_1 = require("./core");
 const ElectronFramework_1 = require("./electron/ElectronFramework");
 const LibUiFramework_1 = require("./frameworks/LibUiFramework");
-const platformPackager_1 = require("./platformPackager");
 const ProtonFramework_1 = require("./ProtonFramework");
 const targetFactory_1 = require("./targets/targetFactory");
-const config_1 = require("./util/config");
+const config_1 = require("./config/config");
 const macroExpander_1 = require("./util/macroExpander");
 const packageDependencies_1 = require("./util/packageDependencies");
 const packageMetadata_1 = require("./util/packageMetadata");
@@ -24,6 +23,7 @@ const repositoryInfo_1 = require("./util/repositoryInfo");
 const yarn_1 = require("./util/yarn");
 const version_1 = require("./version");
 const os_1 = require("os");
+const resolve_1 = require("./util/resolve");
 function addHandler(emitter, event, handler) {
     emitter.on(event, handler);
 }
@@ -207,7 +207,7 @@ class Packager {
             arch: event.arch == null ? null : builder_util_1.Arch[event.arch],
             file: builder_util_1.log.filePath(event.file),
         }, "building");
-        const handler = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, this.config.artifactBuildStarted, "artifactBuildStarted");
+        const handler = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.artifactBuildStarted, "artifactBuildStarted");
         if (handler != null) {
             await Promise.resolve(handler(event));
         }
@@ -219,20 +219,20 @@ class Packager {
         this.eventEmitter.emit("artifactCreated", event);
     }
     async callArtifactBuildCompleted(event) {
-        const handler = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, this.config.artifactBuildCompleted, "artifactBuildCompleted");
+        const handler = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.artifactBuildCompleted, "artifactBuildCompleted");
         if (handler != null) {
             await Promise.resolve(handler(event));
         }
         this.dispatchArtifactCreated(event);
     }
     async callAppxManifestCreated(path) {
-        const handler = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, this.config.appxManifestCreated, "appxManifestCreated");
+        const handler = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.appxManifestCreated, "appxManifestCreated");
         if (handler != null) {
             await Promise.resolve(handler(path));
         }
     }
     async callMsiProjectCreated(path) {
-        const handler = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, this.config.msiProjectCreated, "msiProjectCreated");
+        const handler = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.msiProjectCreated, "msiProjectCreated");
         if (handler != null) {
             await Promise.resolve(handler(path));
         }
@@ -406,7 +406,7 @@ class Packager {
             builder_util_1.log.info({ reason: "npmRebuild is set to false" }, "skipped dependencies rebuild");
             return;
         }
-        const beforeBuild = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, config.beforeBuild, "beforeBuild");
+        const beforeBuild = await (0, resolve_1.resolveFunction)(this.appInfo.type, config.beforeBuild, "beforeBuild");
         if (beforeBuild != null) {
             const performDependenciesInstallOrRebuild = await beforeBuild({
                 appDir: this.appDir,
@@ -433,7 +433,7 @@ class Packager {
         }
     }
     async afterPack(context) {
-        const afterPack = await (0, platformPackager_1.resolveFunction)(this.appInfo.type, this.config.afterPack, "afterPack");
+        const afterPack = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.afterPack, "afterPack");
         const handlers = this.afterPackHandlers.slice();
         if (afterPack != null) {
             // user handler should be last
diff --git a/node_modules/app-builder-lib/out/platformPackager.js b/node_modules/app-builder-lib/out/platformPackager.js
index 9e23ed5..2850398 100644
--- a/node_modules/app-builder-lib/out/platformPackager.js
+++ b/node_modules/app-builder-lib/out/platformPackager.js
@@ -4,7 +4,6 @@ exports.PlatformPackager = void 0;
 exports.isSafeGithubName = isSafeGithubName;
 exports.computeSafeArtifactNameIfNeeded = computeSafeArtifactNameIfNeeded;
 exports.normalizeExt = normalizeExt;
-exports.resolveFunction = resolveFunction;
 exports.chooseNotNull = chooseNotNull;
 const bluebird_lst_1 = require("bluebird-lst");
 const builder_util_1 = require("builder-util");
@@ -12,7 +11,6 @@ const builder_util_2 = require("builder-util");
 const promises_1 = require("fs/promises");
 const lazy_val_1 = require("lazy-val");
 const path = require("path");
-const url_1 = require("url");
 const appInfo_1 = require("./appInfo");
 const asarFileChecker_1 = require("./asar/asarFileChecker");
 const asarUtil_1 = require("./asar/asarUtil");
@@ -24,6 +22,7 @@ const index_1 = require("./index");
 const appBuilder_1 = require("./util/appBuilder");
 const appFileCopier_1 = require("./util/appFileCopier");
 const macroExpander_1 = require("./util/macroExpander");
+const resolve_1 = require("./util/resolve");
 class PlatformPackager {
     get packagerOptions() {
         return this.info.options;
@@ -151,7 +150,7 @@ class PlatformPackager {
         }
         // Due to node-gyp rewriting GYP_MSVS_VERSION when reused across the same session, we must reset the env var: https://github.com/electron-userland/electron-builder/issues/7256
         delete process.env.GYP_MSVS_VERSION;
-        const beforePack = await resolveFunction(this.appInfo.type, this.config.beforePack, "beforePack");
+        const beforePack = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.beforePack, "beforePack");
         if (beforePack != null) {
             await beforePack({
                 appOutDir,
@@ -180,7 +179,7 @@ class PlatformPackager {
             arch: builder_util_1.Arch[arch],
             version: framework.version,
         });
-        const afterExtract = await resolveFunction(this.appInfo.type, this.config.afterExtract, "afterExtract");
+        const afterExtract = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.afterExtract, "afterExtract");
         if (afterExtract != null) {
             await afterExtract({
                 appOutDir,
@@ -265,7 +264,7 @@ class PlatformPackager {
             electronPlatformName: platformName,
         };
         const didSign = await this.signApp(packContext, isAsar);
-        const afterSign = await resolveFunction(this.appInfo.type, this.config.afterSign, "afterSign");
+        const afterSign = await (0, resolve_1.resolveFunction)(this.appInfo.type, this.config.afterSign, "afterSign");
         if (afterSign != null) {
             if (didSign) {
                 await Promise.resolve(afterSign(packContext));
@@ -601,51 +600,6 @@ function computeSafeArtifactNameIfNeeded(suggestedName, safeNameProducer) {
 function normalizeExt(ext) {
     return ext.startsWith(".") ? ext.substring(1) : ext;
 }
-async function resolveModule(type, name) {
-    var _a, _b, _c;
-    const extension = path.extname(name).toLowerCase();
-    const isModuleType = type === "module";
-    try {
-        if (extension === ".mjs" || (extension === ".js" && isModuleType)) {
-            const fileUrl = (0, url_1.pathToFileURL)(name).href;
-            return await eval("import('" + fileUrl + "')");
-        }
-    }
-    catch (error) {
-        builder_util_1.log.debug({ moduleName: name, message: (_a = error.message) !== null && _a !== void 0 ? _a : error.stack }, "Unable to dynamically import hook, falling back to `require`");
-    }
-    try {
-        return require(name);
-    }
-    catch (error) {
-        builder_util_1.log.error({ moduleName: name, message: (_b = error.message) !== null && _b !== void 0 ? _b : error.stack }, "Unable to `require` hook");
-        throw new Error((_c = error.message) !== null && _c !== void 0 ? _c : error.stack);
-    }
-}
-async function resolveFunction(type, executor, name) {
-    if (executor == null || typeof executor !== "string") {
-        return executor;
-    }
-    let p = executor;
-    if (p.startsWith(".")) {
-        p = path.resolve(p);
-    }
-    try {
-        p = require.resolve(p);
-    }
-    catch (e) {
-        (0, builder_util_1.debug)(e);
-        p = path.resolve(p);
-    }
-    const m = await resolveModule(type, p);
-    const namedExport = m[name];
-    if (namedExport == null) {
-        return m.default || m;
-    }
-    else {
-        return namedExport;
-    }
-}
 function chooseNotNull(v1, v2) {
     return v1 == null ? v2 : v1;
 }
diff --git a/node_modules/app-builder-lib/out/util/NodeModuleCopyHelper.js b/node_modules/app-builder-lib/out/util/NodeModuleCopyHelper.js
index 77f8f5b..f61ef0c 100644
--- a/node_modules/app-builder-lib/out/util/NodeModuleCopyHelper.js
+++ b/node_modules/app-builder-lib/out/util/NodeModuleCopyHelper.js
@@ -6,7 +6,7 @@ const builder_util_1 = require("builder-util");
 const fs_extra_1 = require("fs-extra");
 const path = require("path");
 const fileMatcher_1 = require("../fileMatcher");
-const platformPackager_1 = require("../platformPackager");
+const resolve_1 = require("./resolve");
 const AppFileWalker_1 = require("./AppFileWalker");
 const fs_1 = require("fs");
 const excludedFiles = new Set([".DS_Store", "node_modules" /* already in the queue */, "CHANGELOG.md", "ChangeLog", "changelog.md", "Changelog.md", "Changelog", "binding.gyp", ".npmignore"].concat(fileMatcher_1.excludedNames.split(",")));
@@ -37,7 +37,7 @@ class NodeModuleCopyHelper extends AppFileWalker_1.FileCopyHelper {
         var _a;
         const filter = this.filter;
         const metadata = this.metadata;
-        const onNodeModuleFile = await (0, platformPackager_1.resolveFunction)(this.packager.appInfo.type, this.packager.config.onNodeModuleFile, "onNodeModuleFile");
+        const onNodeModuleFile = await (0, resolve_1.resolveFunction)(this.packager.appInfo.type, this.packager.config.onNodeModuleFile, "onNodeModuleFile");
         const result = [];
         const queue = [];
         const emptyDirs = new Set();
diff --git a/node_modules/app-builder-lib/out/util/resolve.js b/node_modules/app-builder-lib/out/util/resolve.js
new file mode 100644
index 0000000..8965d2c
--- /dev/null
+++ b/node_modules/app-builder-lib/out/util/resolve.js
@@ -0,0 +1,54 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.resolveModule = resolveModule;
+exports.resolveFunction = resolveFunction;
+const log_1 = require("builder-util/out/log");
+const debug_1 = require("debug");
+const path = require("path");
+const url_1 = require("url");
+async function resolveModule(type, name) {
+    var _a, _b, _c;
+    const extension = path.extname(name).toLowerCase();
+    const isModuleType = type === "module";
+    try {
+        if (extension === ".mjs" || (extension === ".js" && isModuleType)) {
+            const fileUrl = (0, url_1.pathToFileURL)(name).href;
+            return await eval("import('" + fileUrl + "')");
+        }
+    }
+    catch (error) {
+        log_1.log.debug({ moduleName: name, message: (_a = error.message) !== null && _a !== void 0 ? _a : error.stack }, "Unable to dynamically import, falling back to `require`");
+    }
+    try {
+        return require(name);
+    }
+    catch (error) {
+        log_1.log.error({ moduleName: name, message: (_b = error.message) !== null && _b !== void 0 ? _b : error.stack }, "Unable to `require`");
+        throw new Error((_c = error.message) !== null && _c !== void 0 ? _c : error.stack);
+    }
+}
+async function resolveFunction(type, executor, name) {
+    if (executor == null || typeof executor !== "string") {
+        return executor;
+    }
+    let p = executor;
+    if (p.startsWith(".")) {
+        p = path.resolve(p);
+    }
+    try {
+        p = require.resolve(p);
+    }
+    catch (e) {
+        (0, debug_1.default)(e);
+        p = path.resolve(p);
+    }
+    const m = await resolveModule(type, p);
+    const namedExport = m[name];
+    if (namedExport == null) {
+        return m.default || m;
+    }
+    else {
+        return namedExport;
+    }
+}
+//# sourceMappingURL=resolve.js.map
\ No newline at end of file
jbool24 commented 2 months ago

@mmaietta will do. I'll have a chance to check it out this after (EST) and then follow up. 👍

jbool24 commented 2 months ago

@mmaietta

Well I applied the patch and compiled electron with success (no warnings). What exactly should I be on the lookout for. I have the following in my current workaround setup for electron-builder to work prior to this patch.

  1. electron-builder.config.cjs -- note file type as *.cjs
  2. with type: module in package.json
  3. also the electron preloader file is also specifically named with *.mjs.

What should I change in order to test this patch? Is this updated intended to allow for all files to just be *.js files when type: module is set in the package.json?

mmaietta commented 2 months ago

Correct, you can now use .js config file with type: module in package.json

We are reusing the resolveModule logic (same that applies to hooks) that determines require/import based on this logic

    if (extension === ".mjs" || (extension === ".js" && isModuleType)) {
      const fileUrl = pathToFileURL(name).href
      return await eval("import('" + fileUrl + "')")
    }

isModuleType comes from type:module in package.json

If any part of this logic fails, it'll throw a debug and/or error in the console or fail to import completely (likely with the electron-builder.config.js is treated as an ES module file as it is a .js file error message) https://github.com/electron-userland/electron-builder/blob/5c8373d15f99ee9a4c46ed164f95bf1d4a11db28/packages/app-builder-lib/src/util/resolve.ts#L6-L23

To test:

  1. electron-builder.config.js
  2. with type: module in package.json

config loading comes from here, so maybe it automatically works with .cjs extension anyhow, but it's using require instead of eval("import('" + fileUrl + "')") https://github.com/electron-userland/electron-builder/blob/5c8373d15f99ee9a4c46ed164f95bf1d4a11db28/packages/app-builder-lib/src/util/config/load.ts#L21-L24

jbool24 commented 2 months ago

Got it. So changing the files back to *.js with type:module in package.json and import statements in the configs we get the following error:

Using patch-package@8.0.0 to apply the patchfile app-builder-lib+25.0.5.patch from above

$ electron-builder build --config electron-builder.config.js
  • electron-builder  version=25.0.5 os=6.8.0-40-generic
  ⨯ Unable to `require`  moduleName=/{{REDACTED DIR PATH}}/electron-builder.config.js message=require() of ES Module /{{REDACTED DIR PATH}}/electron-builder.config.js from /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js not supported.
Instead change the require of electron-builder.config.js in /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js to a dynamic import() which is available in all CommonJS modules.
  ⨯ require() of ES Module /{{REDACTED DIR PATH}}/electron-builder.config.js from /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js not supported.
Instead change the require of electron-builder.config.js in /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js to a dynamic import() which is available in all CommonJS modules.  failedTask=build stackTrace=Error: require() of ES Module /{{REDACTED DIR PATH}}/electron-builder.config.js from /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js not supported.
Instead change the require of electron-builder.config.js in /{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js to a dynamic import() which is available in all CommonJS modules.
    at resolveModule (/{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/util/resolve.js:27:15)
    at readConfig (/{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/config/loadConfig.js:26:18)
    at getConfig (/{{REDACTED DIR PATH}}/node_modules/app-builder-lib/out/config/config.js:38:36)
    at Packager.validateConfig (/{{REDACTED DIR PATH}}/node_modules/app-builder-lib/src/packager.ts:330:27)
    at Packager.build (/{{REDACTED DIR PATH}}/node_modules/app-builder-lib/src/packager.ts:361:5)
    at executeFinally (/{{REDACTED DIR PATH}}/node_modules/builder-util/src/promise.ts:12:14)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
yarn compile  19.85s user 1.31s system 172% cpu 12.241 total
mmaietta commented 2 months ago

Hmmm, I'm unable to reproduce that on my end off latest master, which has this implementation. Might just have to release and have you retest 😅 . Do you have a monorepo or multi-package.json structure? Wondering if this is pulling the wrong package.json that has type: module in it

fs.readFile(path.join(process.cwd(), "package.json"), "utf8")
jbool24 commented 2 months ago

@mmaietta Nope I'm not using a multi-repo or monorepo setup. But it is a TS project compiled by Vite/Rollup. It might be something with that configuration. But only one single top level package.json. I'm gonna try a few more things tomorrow.

mmaietta commented 2 months ago

Hmmm, the TS compilation shouldn't impact the loading of the config file unless you're also compiling the electron-builder config file prior to electron-builder loading it.

In any case, I was able to test successfully for both electron-builder configs with (.ts, .cjs, .js, and .mjs using module.exports or export default depending on type=module or not) and the CI tests are still passing.

Just pushed the release commit, so a new deployment for 25.0.6 should occur in the next 10min.

mmaietta commented 2 months ago

@jbool24 please try 25.0.6 with DEBUG=electron-builder in case any debug messages are thrown

jbool24 commented 2 months ago

@mmaietta I'll have an opportunity to test again tomorrow morning. I'll use the DEBUG env flag to see if any other stuff spits out and report back. It may just be my existing setup that I'm testing it in. I will have time tomorrow to use a vanilla base project to sanity check that theory as well.