Skip to content

feat: add support for packaging fiddles as ASAR using @electron/asar #138

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

Merged
merged 16 commits into from
Apr 7, 2025

Conversation

AlokYadavCodes
Copy link
Contributor

@AlokYadavCodes AlokYadavCodes commented Mar 14, 2025

  • Introduces a new optional option parameter in FiddleFactory.create() method.
  • When { packAsAsar: true } is passed as option, the fiddle is packaged as an ASAR using @electron/asar.
  • Updates tests to cover the ASAR packaging functionality.

Closes #100

@AlokYadavCodes AlokYadavCodes requested a review from a team as a code owner March 14, 2025 18:30
@AlokYadavCodes AlokYadavCodes force-pushed the feat/100-fiddle-asar-pack branch from d24be4b to c97f17c Compare March 14, 2025 18:39
@AlokYadavCodes AlokYadavCodes force-pushed the feat/100-fiddle-asar-pack branch from c97f17c to c791fab Compare March 14, 2025 19:13
@AlokYadavCodes
Copy link
Contributor Author

Hi @erickzhao, just a friendly reminder for a review when you have a moment.
Let me know if any changes are required.

@erickzhao
Copy link
Member

erickzhao commented Mar 31, 2025

Pushed up a few commits here due to the main branch being fixed (3b420d8) and to re-commit the lockfile due to our internal policies around that.

@dsanders11
Copy link
Member

This looks like a good start, but I think some changes are needed before we can merge. I've left a comment about the runFromAsar option, and we'll also want to add test coverage in tests/runner.test.ts to ensure the option works as expected.

@AlokYadavCodes
Copy link
Contributor Author

we'll also want to add test coverage in tests/runner.test.ts to ensure the option works as expected.

Added a test for runFromAsar in runner.test.ts, though not fully sure if it's what you had in mind. Let me know if it needs changes!

Copy link
Member

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

This is close, just a few more comments. Be sure to run yarn docs after the final changes to ensure that etc/fiddle-core.api.md is up-to-date.

@dsanders11 dsanders11 merged commit 8c7f685 into electron:main Apr 7, 2025
13 checks passed
Copy link

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Run from ASAR Support
3 participants