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.

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

feat: allow usage of .cjs, .mjs, and type=module custom publishers #8868

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

mmaietta
Copy link
Collaborator

@mmaietta mmaietta commented Feb 17, 2025

Resolves: #8862

Adds AsyncEventEmitter:

  • allows listeners to be queued as Promises and resolved during emit
  • event name type-safety
  • Consolidates a majority of resolveFunction for user hooks to the beginning of the packager build process so users can detect errors early if import is incorrect

Adds unit test to cover all core user-provided hooks

Copy link

changeset-bot bot commented Feb 17, 2025

🦋 Changeset detected

Latest commit: d9878fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
app-builder-lib Patch
dmg-builder Patch
electron-builder-squirrel-windows Patch
electron-builder Patch
electron-forge-maker-appimage Patch
electron-forge-maker-nsis-web Patch
electron-forge-maker-nsis Patch
electron-forge-maker-snap Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mmaietta mmaietta marked this pull request as ready for review February 17, 2025 02:12
@mmaietta mmaietta requested a review from beyondkmp February 18, 2025 00:08
@@ -288,36 +288,36 @@ File `myBeforePackHook.js` in the project root directory:
}
```
*/
readonly beforePack?: Hook<BeforePackContext, any> | string | null
readonly beforePack?: Hook<BeforePackContext, void> | string | null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second parameter for all Hooks is void. Given this, should we consider removing this void parameter for now and only keep the first parameter? We can always add it back later if needed.

this.eventEmitter.on("beforePack", resolveFunction(type, this.config.beforePack, "beforePack"), "user")
this.eventEmitter.on("afterExtract", resolveFunction(type, this.config.afterExtract, "afterExtract"), "user")
this.eventEmitter.on("afterPack", resolveFunction(type, this.config.afterPack, "afterPack"), "user")
this.eventEmitter.on("afterSign", resolveFunction(type, this.config.afterSign, "afterSign"), "user")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we consider introducing a state machine in the future? If a certain state fails, it could revert to the previous state and retry?

@@ -132,6 +132,9 @@ export function build(options: PackagerOptions & PublishOptions, packager: Packa
promise = publishManager.awaitTasks()
}

return promise.then(() => process.removeListener("SIGINT", sigIntHandler))
return promise.then(() => {
packager.clearPackagerEventListeners()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any one of the states fails, will it be released here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Allow custom publisher with .cjs Extension
2 participants