Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Feature Request] Allow custom publisher with .cjs Extension #8862

Open
davbrito opened this issue Feb 11, 2025 · 2 comments · May be fixed by #8868
Open

[Feature Request] Allow custom publisher with .cjs Extension #8862

davbrito opened this issue Feb 11, 2025 · 2 comments · May be fixed by #8868

Comments

@davbrito
Copy link

davbrito commented Feb 11, 2025 •
edited
Loading

Description

I would like to request support for using the .cjs extension for build/electron-publisher-custom.cjs files. This would help in scenarios where you have "type": "module" in your package.json. Additionally, it would be beneficial to support ES modules for the custom publisher file.

Current Behavior

Currently, it only accepts .js files with CommonJS. So, it ignores the .cjs file. But, if I use .js it would fail to load. Because I have "type": "module" in the package.json.

Proposed Behavior

  • Extend the support to include the .cjs extension for custom publisher scripts, allowing files named build/electron-publisher-custom.cjs to be used.
  • Consider allowing the custom publisher file to be an ES module. This can be achieved by importing it with a dynamic import in your code.

Use Case

This feature is particularly useful for projects that use "type": "module" and need custom publisher scripts in the Electron build process. Supporting ES modules would further increase flexibility and adherence to modern JavaScript standards.

Key Line Reference

The relevant key line is located here.

electron-builder/packages/app-builder-lib/src/publish/PublishManager.ts

Lines 352 to 356 in f4fc04a

try {
module = require(path.join(packager.buildResourcesDir, name + ".js"))
} catch (_ignored) {
log.debug({ path: path.join(packager.buildResourcesDir, name + ".js") }, "Unable to find publish provider in build resources")
}

Benefits

Increased flexibility in the types of modules that can be used for custom publishers.
Better alignment with Node.js module standards.
Support for modern ES modules, enhancing compatibility and future-proofing.
@mmaietta
Copy link
Collaborator

mmaietta commented Feb 11, 2025 •
edited
Loading

Great request, I think we have already-existing logic that we can leverage here to make this feature request come true.
https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/util/resolve.ts#L6-L23
This supports module type scripts as well as cjs, albeit looks like .cjs isn't respected atm.

I'll take a look at this when I find some free time. Just quick note, I do have some high-prio bugs to get to first though.

@mmaietta
Copy link
Collaborator

@davbrito would you be willing to try this patch-package out and see if it works for your setup? Most notably, the only thing I'm worried about is the order with which uploads are scheduled, but that shouldn't matter AFAICT. This should support .cjs, .js(cjs and module), and .mjs
app-builder-lib+26.0.6.patch

diff --git a/node_modules/app-builder-lib/out/publish/PublishManager.js b/node_modules/app-builder-lib/out/publish/PublishManager.js
index 3c6a845..7670806 100644
--- a/node_modules/app-builder-lib/out/publish/PublishManager.js
+++ b/node_modules/app-builder-lib/out/publish/PublishManager.js
@@ -18,6 +18,8 @@ const url = require("url");
 const index_1 = require("../index");
 const macroExpander_1 = require("../util/macroExpander");
 const updateInfoBuilder_1 = require("./updateInfoBuilder");
+const fs_extra_1 = require("fs-extra");
+const resolve_1 = require("../util/resolve");
 const publishForPrWarning = "There are serious security concerns with PUBLISH_FOR_PULL_REQUEST=true (see the  CircleCI documentation (https://circleci.com/docs/1.0/fork-pr-builds/) for details)" +
     "\nIf you have SSH keys, sensitive env vars or AWS credentials stored in your project settings and untrusted forks can make pull requests against your repo, then this option isn't for you.";
 const debug = (0, debug_1.default)("electron-builder:publish");
@@ -95,7 +97,7 @@ class PublishManager {
                 if (debug.enabled) {
                     debug(`artifactCreated (isPublish: ${this.isPublish}): ${(0, builder_util_1.safeStringifyJson)(event, new Set(["packager"]))},\n  publishConfig: ${(0, builder_util_1.safeStringifyJson)(publishConfiguration)}`);
                 }
-                this.scheduleUpload(publishConfiguration, event, this.getAppInfo(event.packager));
+                void this.scheduleUpload(publishConfiguration, event, this.getAppInfo(event.packager));
             }
         });
     }
@@ -106,11 +108,11 @@ class PublishManager {
         const publishers = this.packager.config.publish;
         return await resolvePublishConfigurations(publishers, null, this.packager, null, true);
     }
-    scheduleUpload(publishConfig, event, appInfo) {
+    async scheduleUpload(publishConfig, event, appInfo) {
         if (publishConfig.provider === "generic") {
             return;
         }
-        const publisher = this.getOrCreatePublisher(publishConfig, appInfo);
+        const publisher = await this.getOrCreatePublisher(publishConfig, appInfo);
         if (publisher == null) {
             builder_util_1.log.debug({
                 file: builder_util_1.log.filePath(event.file),
@@ -149,7 +151,7 @@ class PublishManager {
                     builder_util_1.log.debug({ file: event.file, reason: "cancelled" }, "not published");
                     break;
                 }
-                this.scheduleUpload(publishConfig, event, this.getAppInfo(platformPackager));
+                await this.scheduleUpload(publishConfig, event, this.getAppInfo(platformPackager));
             }
         }
         if (event.isWriteUpdateInfo &&
@@ -160,12 +162,12 @@ class PublishManager {
             this.taskManager.addTask((0, updateInfoBuilder_1.createUpdateInfoTasks)(event, publishConfigs).then(it => this.updateFileWriteTask.push(...it)));
         }
     }
-    getOrCreatePublisher(publishConfig, appInfo) {
+    async getOrCreatePublisher(publishConfig, appInfo) {
         // to not include token into cache key
         const providerCacheKey = (0, builder_util_1.safeStringifyJson)(publishConfig);
         let publisher = this.nameToPublisher.get(providerCacheKey);
         if (publisher == null) {
-            publisher = createPublisher(this, appInfo.version, publishConfig, this.publishOptions, this.packager);
+            publisher = await createPublisher(this, appInfo.version, publishConfig, this.publishOptions, this.packager);
             this.nameToPublisher.set(providerCacheKey, publisher);
             builder_util_1.log.info({ publisher: publisher.toString() }, "publishing");
         }
@@ -225,7 +227,7 @@ async function getPublishConfigsForUpdateInfo(packager, publishConfigs, arch) {
     }
     return publishConfigs;
 }
-function createPublisher(context, version, publishConfig, options, packager) {
+async function createPublisher(context, version, publishConfig, options, packager) {
     if (debug.enabled) {
         debug(`Create publisher: ${(0, builder_util_1.safeStringifyJson)(publishConfig)}`);
     }
@@ -240,12 +242,12 @@ function createPublisher(context, version, publishConfig, options, packager) {
         case "generic":
             return null;
         default: {
-            const clazz = requireProviderClass(provider, packager);
+            const clazz = await requireProviderClass(provider, packager);
             return clazz == null ? null : new clazz(context, publishConfig);
         }
     }
 }
-function requireProviderClass(provider, packager) {
+async function requireProviderClass(provider, packager) {
     switch (provider) {
         case "github":
             return electron_publish_1.GitHubPublisher;
@@ -262,18 +264,14 @@ function requireProviderClass(provider, packager) {
         case "bitbucket":
             return electron_publish_1.BitbucketPublisher;
         default: {
-            const name = `electron-publisher-${provider}`;
-            let module = null;
-            try {
-                module = require(path.join(packager.buildResourcesDir, name + ".js"));
-            }
-            catch (_ignored) {
-                builder_util_1.log.debug({ path: path.join(packager.buildResourcesDir, name + ".js") }, "Unable to find publish provider in build resources");
-            }
-            if (module == null) {
-                module = require(name);
+            const name = (ext) => `electron-publisher-${provider}.${ext}`;
+            const validPublisherFiles = [".mjs", ".js", ".cjs"].map(ext => name(ext));
+            for (const publisherFilename of validPublisherFiles) {
+                const potentialFile = path.join(packager.buildResourcesDir, publisherFilename);
+                if ((0, fs_extra_1.existsSync)(potentialFile)) {
+                    return await (0, resolve_1.resolveModule)(packager.appInfo.type, potentialFile);
+                }
             }
-            return module.default || module;
         }
     }
 }
@@ -392,7 +390,7 @@ async function getResolvedPublishConfig(platformPackager, packager, options, arc
         }
         return options;
     }
-    const providerClass = requireProviderClass(options.provider, packager);
+    const providerClass = await requireProviderClass(options.provider, packager);
     if (providerClass != null && providerClass.checkAndResolveOptions != null) {
         await providerClass.checkAndResolveOptions(options, channelFromAppVersion, errorIfCannot);
         return options;

@mmaietta mmaietta linked a pull request Feb 17, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mmaietta @davbrito