[X] I agree to follow the code of conduct that this project follows, as appropriate.
[ ] The changes are appropriately documented (if applicable).
[ ] The changes have sufficient test coverage (if applicable).
[X] The testsuite passes successfully on my local machine (if applicable).
Summary
This PR focuses on bringing the initial support loading Forge modules (plugins, makers, publishers etc.) that use ESM for their module format. It should also contain some cosmetic changes like rename of require-search to import-search (to reflect it isn't around the require API only anymore), improvements around the helper so it won't resolve path for modules passed by their names and introduces new function, dynamicImportMaybe, that will also try doing require first and falling back to ESM when it fails (I chose to require first so I won't have to resolve default in CJS import).
The main motivation on that is to allow third-parties as well as official Forge modules to be able to effortlessly utilise the ESM syntax entirely. While this is possible for now with the Forge as well, it requires of already importing the class to the config on your own.
There are currently a few things that could be considered to be worked on in the future, but are not really necessary from what I've been testing:
Resolving directory paths for import in order to load them properly. This could be useful when makers are installed in unusual paths that even make Node not to be able to find modules when they are placed that way. Plus, there could be some issue with actually finding what to import given how complex it became in Node to resolve imports, plus resolving internal imports (starting in #) would probably also have to be supported as well. It might not be worth of the efforts for now.
(Maybe) dropping dynamicImport or making it private, since it doesn't seem to be used outside of its own module scope anymore.
Improving some tests, so they have simpler logic. Currently, I focused more on preserving on the logic that tests actually uses and only changing them by making them async or use some wrapped stuff. This should make them work essentially similar way while fixing false negatives introduced by some drastic changes, like making some synchronous functions async or renaming a module.Done in 9229bff.
Of course, due to the changes, I had to modify tests for require-search and publish-spec, due to changes in module names and switching some previously synchronous code to be async. For now, testing only the workspace @electron-forge/core, I've seen similar number of tests failing on both official implementation and this fork, and I think they were caused because Forge expected to run tests from a root project than a workspace, as I saw those were mostly caused due to missing scripts in workspace package (so it's nothing serious). I guess it's out of the scope to fix that tho.
Additional notes
I hope my commit messages as well as their classification and even this PR name fits well the policy of this project. This is my first time to contribute to Forge, so I'm definitely learning how to do stuff right while trying to avoid mistakes as much as I could 😄️.
Summary
This PR focuses on bringing the initial support loading Forge modules (plugins, makers, publishers etc.) that use ESM for their module format. It should also contain some cosmetic changes like rename of
require-searchtoimport-search(to reflect it isn't around therequireAPI only anymore), improvements around the helper so it won't resolve path for modules passed by their names and introduces new function,dynamicImportMaybe, that will also try doingrequirefirst and falling back to ESM when it fails (I chose torequirefirst so I won't have to resolvedefaultin CJS import).The main motivation on that is to allow third-parties as well as official Forge modules to be able to effortlessly utilise the ESM syntax entirely. While this is possible for now with the Forge as well, it requires of already importing the class to the config on your own.
There are currently a few things that could be considered to be worked on in the future, but are not really necessary from what I've been testing:
Resolving directory paths for
importin order to load them properly. This could be useful when makers are installed in unusual paths that even make Node not to be able to find modules when they are placed that way. Plus, there could be some issue with actually finding what to import given how complex it became in Node to resolve imports, plus resolving internal imports (starting in#) would probably also have to be supported as well. It might not be worth of the efforts for now.(Maybe) dropping
dynamicImportor making it private, since it doesn't seem to be used outside of its own module scope anymore.Improving some tests, so they have simpler logic. Currently, I focused more on preserving on the logic that tests actually uses and only changing them by making them async or use some wrapped stuff. This should make them work essentially similar way while fixing false negatives introduced by some drastic changes, like making some synchronous functions async or renaming a module.Done in 9229bff.I should also mention, the changes in this repo were tested by me (locally) with my own implementation of ESM maker.
Changes around the tests
Of course, due to the changes, I had to modify tests for
require-searchandpublish-spec, due to changes in module names and switching some previously synchronous code to be async. For now, testing only the workspace@electron-forge/core, I've seen similar number of tests failing on both official implementation and this fork, and I think they were caused because Forge expected to run tests from a root project than a workspace, as I saw those were mostly caused due to missing scripts in workspace package (so it's nothing serious). I guess it's out of the scope to fix that tho.Additional notes
I hope my commit messages as well as their classification and even this PR name fits well the policy of this project. This is my first time to contribute to Forge, so I'm definitely learning how to do stuff right while trying to avoid mistakes as much as I could 😄️.