fiddle icon indicating copy to clipboard operation
fiddle copied to clipboard

feat: add run from ASAR button

Open AlokYadavCodes opened this issue 11 months ago • 9 comments

  • Adds a dropdown to the right of the "Run" button, featuring a new button labeled "Run from ASAR"
  • Introduces a new optional parameter runOptions in Runner.run() method.
  • The runOptions parameter is defined as follows:
interface runOptions {
  runFromAsar: boolean;
}
  • Updates test snapshots to reflect the new UI changes.

Updated UI screenshots:

Screenshot from 2025-03-20 23-48-01 Screenshot from 2025-03-20 23-48-16

Closes #1551

This functionality requires an upstream change in @electron/fiddle-core.

I have already opened a PR to implement the required updates in fiddle-core:

Please review both the PR's and let me know if any changes are needed.

AlokYadavCodes avatar Mar 20 '25 19:03 AlokYadavCodes

Hey @codebytere , just checking in. Let me know if there's anything I can do to help with the review.

Also tagging @dsanders11 , since you opened this issue and are mentoring the GSoC project I’m applying for (Fiddle Support for Fuses), I would love to hear your thoughts as well.

Thanks so much for your time and guidance. I really appreciate it and am excited to incorporate any feedback you might have!

AlokYadavCodes avatar Mar 23 '25 06:03 AlokYadavCodes

@AlokYadavCodes, I've changed this PR to be a draft for the moment, the test failures are related to needing the @electron/fiddle-core PR to land first.

dsanders11 avatar Apr 04 '25 23:04 dsanders11

Hey @dsanders11 , Now that the fiddle-core PR is merged, this one should be ready for review whenever you get a chance.

Also, I submitted a draft GSoC proposal titled "Electron Fiddle Support for Fuses" on April 2. If you have a moment to share any quick feedback before the deadline tomorrow, I'd really appreciate it. Totally understand if you're busy - thanks!

AlokYadavCodes avatar Apr 07 '25 16:04 AlokYadavCodes

@AlokYadavCodes, you'll need to bump the version of @electron/fiddle-core in this PR to use the new v1.4.0 release with your corresponding changes there.

If you have a moment to share any quick feedback before the deadline tomorrow, I'd really appreciate it. Totally understand if you're busy - thanks!

Please send it to [email protected] as that's where we provide draft feedback. šŸ™‚

dsanders11 avatar Apr 07 '25 18:04 dsanders11

@dsanders11 Updated the PR to use @electron/[email protected] and sent the draft proposal to the email you mentioned. Thanks again for the guidance!

AlokYadavCodes avatar Apr 07 '25 20:04 AlokYadavCodes

Coverage Status

coverage: 87.224% (-0.2%) from 87.472% when pulling 15a7597380866512e1a39d85fe1a2ce3dcaff466 on AlokYadavCodes:feat/1551-run-from-asar into d5e133f85426cd5a1354d24d670606afa418bc8a on electron:main.

coveralls avatar Apr 07 '25 20:04 coveralls

@AlokYadavCodes, the UI around the "Run from ASAR" button needs to be improved from my testing, the dropdown is a bit awkward looking with the focus ring around it (could it look more like the the dropdown for public/private gist?) and the dropdown remains there after I've clicked on it.

I've also realized that this line is preventing this from working (due to starting options with dir) as we're actually running the fiddle out of the copied files, rather than the directory that fiddle-core creates. This wasn't caught in the refactor to start using fiddle-core internally since it just works, but it'll need to be changed to ensure the ASAR is used. I need to think through any other consequences of changing this before we do it, but you can work on the UI improvements in the meantime.

https://github.com/electron/fiddle/blob/d5e133f85426cd5a1354d24d670606afa418bc8a/src/renderer/runner.ts#L370

dsanders11 avatar Apr 07 '25 22:04 dsanders11

@dsanders11, fixed the UI to match the dropdown style used for public/private gist selection.

I've also realized that this line is preventing this from working (due to starting options with dir) as we're actually running the fiddle out of the copied files, rather than the directory that fiddle-core creates.

Should we also update the cwd in child_process.spawn() options to point to the directory created by fiddle-core? Currently it is set to the copied files directory path. https://github.com/electron/fiddle/blob/53057ba0fd63b52c94f7aed9b60c810ec55cf92c/src/main/fiddle-core.ts#L56

AlokYadavCodes avatar Apr 10 '25 08:04 AlokYadavCodes

@dsanders11 Just a gentle reminder about this PR whenever you have a chance. Really appreciate your time. Thanks!

AlokYadavCodes avatar Apr 21 '25 17:04 AlokYadavCodes