feat: add support for ESM presets

[sheerlox]

This PR adds support for loading pure ESM presets. This was not previously possible because import-from uses createRequire under the hood.

In order to replace it, I've created an ES module loader that abstracts the preset loading logic for semantic-release repos (first tries to hit node_modules, then if not found tries to hit the relative path from the current working directory).

I've tested these changes locally on the https://github.com/insurgent-lab/conventional-changelog-preset repo (which is a preset but also uses its current version for releasing with semantic-release), both with the current latest version and the ESM version (which you can find in the refactor/esm branch since it's probably the only conventional-changelog ESM preset at this time).

Related

• feat: add support for ESM presets commit-analyzer#537
• ES Modules semantic-release#2133
• ES Module #296
• discussion on refactor(esm): converted the package to esm commit-analyzer#297 (comment)

cc @travi 😉

[travi]

these are great PRs and highlight the duplication we have for loading across these two plugins (which i'd like to resolve at some point).

I wont be able to take a close look for a few days, but should be able to take a look friday at the latest. the one thing that i'm wondering without a deeper look is if there would be a way to cover the full loading of a cjs preset and esm preset with an integration test. it may be easier to do this in the core repo, but i wanted to put the thought out there in case you'd already considered options about this before i can look deeper.

[sheerlox]

these are great PRs and highlight the duplication we have for loading across these two plugins (which i'd like to resolve at some point).

are you thinking about creating a separate @semantic-release/conventional-preset-loader package to centralize the module-loader.js file into a single repository?

the one thing that i'm wondering without a deeper look is if there would be a way to cover the full loading of a cjs preset and esm preset with an integration test. it may be easier to do this in the core repo, but i wanted to put the thought out there in case you'd already considered options about this before i can look deeper.

I could work on a PR on the core repo to add that kind of test if you confirm that's the best option, let me know!

I wont be able to take a close look for a few days, but should be able to take a look friday at the latest.

not in a hurry, I'm just glad I could find some time to work on this since we talked about it a few months ago 😄

[sheerlox]

While doing research today, I found a "corner" case I didn't test that should fail with this PR.

The issue is with loading a module from the parent module node_modules: "dependencies (without an explicit path) for a given module are searched for relative to the module loading them" (source).

That means the current code probably won't be able to load conventional-commit presets installed along semantic-release.

I didn't run into this issue while testing, since I'm using a relative path to my local preset, and test suites cannot detect that bug since they are using their own node_modules folder.

I'm working on a solution.

[travi]

are you thinking about creating a separate @semantic-release/conventional-preset-loader package to centralize the module-loader.js file into a single repository?

yeah, i'm definitely leaning in this direction, but that would be a future step. no need to worry about this at this point other than maybe trying to be careful to keep the two current implementations as similar as possible and call out where they differ and how necessary any differences are.

I could work on a PR on the core repo to add that kind of test if you confirm that's the best option, let me know!

i think that is the easiest place to add them today, but it has the downside of not catching issues locally within this package before publishing. with the additional use case you mentioned most recently, that highlights even further that an integration test that truly loaded the configs could be a helpful feedback loop. i wont hold up getting this merged based on existence of that type of test, but would be open to the addition if you think it helps give you feedback that would give you more confidence with the solution. if you decide it is worth it, let me know how i can be helpful

[sheerlox]

after encountering the error with importing a module from multiple levels above, I realized the task at hand was a bit more complex than expected and that it might be useful to others, so I decided to port import-from to support importing ES modules (by using import instead of require) while maintaining the same module resolution strategy require uses. I introduced only one breaking change, which is the need for await (since import is asynchronous).

the package also has the ability to load files regardless of their extension (.js, .mjs, .cjs), note they still need to have the correct type depending on the module attribute of their closest package.json (i.e. importing a JS file containing a CJS module while "type": "module" is set will throw an error). this was necessary since with resolve the file extension was optional.

I've tested this version of the PRs with 4 different configurations: CJS /or/ ESM preset /and/ relative path /or/ npm module specifier.

source for the new package is available at sheerlox/import-from-esm, please let me know what you think of this solution.

[sheerlox]

creating a @semantic-release/conventional-preset-loader package is a good call: while commit-analyzer/load-release-rules.js is on its own, the code and tests between changelog-notes-generator/load-changelog-config.js and commit-analyzer/load-parser-config.js are the same, with writerOpts added in load-changelog-config.js (so commit-analyzer could use changelog-notes-generator's implementation. grouping everything would greatly improve consistency and testing, and reduce duplication.

i wont hold up getting this merged based on existence of that type of test, but would be open to the addition if you think it helps give you feedback that would give you more confidence with the solution.

I think that type of test would be a great addition, and indeed would add to my confidence for this PR. but this is going to take time, and since I've been kicking issues down the road for weeks, I'd prefer if you could raise an issue to remember that needs to be done if that's okay for you. I've extensively tested the solution I'm proposing and I'm feeling confident about it.

I always monitor my GH notifications closely, and I'll free some time right away if work needs to be done either on import-from-esm or @semantic-release packages after merging these PRs.

[sheerlox]

Note: I've made significant efforts to get the best ossf/scorecard score possible for import-from-esm, and the package is currently getting a score of 8.2.
The two scopes I cannot improve on for now are Code-Review and Maintained, since I'm the only maintainer and the repository is only 9 days old.
If someone from @semantic-release would like to be invited as a maintainer of the repository, please just let me know and consider it done.

[travi]

sorry that i've been slow to get back to these. i dont think i'm going to get a chance today, but these are still on my radar

[travi]

source for the new package is available at sheerlox/import-from-esm, please let me know what you think of this solution.

i like the solution and appreciate that you've mentioned it in the thread for the original project.

there is one detail about the package implementation that i'm curious about. it looks like you've vendored a manually tree-shaken version of import-meta-resolve. can you help me understand the benefit of vendoring it that way? the downside that stands out to me is that pulling in a potential vulnerability patch would now be more manual than getting an automated update PR from dependabot or renovate (and consumers would also not get a notification as easily that a vulnerability exists).

[sheerlox]

There are two reasons for this choice:

• the function I was interested in (packageResolve) is not exposed by import-meta-resolve
• since my package only uses part of import-meta-resolve, it also helps to reduce package size

I see two different ways of resolving the issues you mentioned:

• ask the import-meta-resolve's maintainer if exposing packageResolve would be okay, propose a PR, and use the package directly. That would increase the import-from-esm package size by 20,2 kB, but I guess that's not a big deal given the security benefits you mentioned
• setup an automatic update pipeline to extract packageResolve from import-meta-resolve

The first solution indeed seems to be the most straightforward and durable/stable solution. Please let me know your thoughts on this, and I'll raise an issue ASAP to discuss that with the maintainer.

[travi]

• the function I was interested in (packageResolve) is not exposed by import-meta-resolve

that makes sense. i overlooked this detail. i dont want a suggestion to improve that to hold up these PRs. if you could update the PRs with mainline once more to resolve conflicts, i'm good with getting these merged. they are great contributions and get us past what i think is our last ESM hurdle in core functionality.

• ask the import-meta-resolve's maintainer if exposing packageResolve would be okay, propose a PR, and use the package directly. That would increase the import-from-esm package size by 20,2 kB, but I guess that's not a big deal given the security benefits you mentioned

this seems worthwhile to at least ask about. i wouldnt be concerned about the additional size, honestly. that is such a small difference and being more easily aware of potential vulnerabilities and general improvements greatly outweigh the size benefit, imho. if the answer is no for that package, it could make sense to continue to vendor, but would be interesting to hear their answer.

[sheerlox]

Raised wooorm/import-meta-resolve#18 to ask if exporting the packageResolve function would be possible, I'll follow up on that and release a new version whenever possible if the maintainer agrees on the changes. Otherwise, I'll find an automation solution.

[github-actions]

🎉 This PR is included in version 12.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[sheerlox]

Turns out exporting the packageResolve function wasn't necessary, I successfully replaced the vendored version of import-meta-resolve with a package dependency and used their moduleResolve function in sheerlox/import-from-esm#35. I've added extensive tests beforehand in sheerlox/import-from-esm#34.
This is going to be released as v1.1.0, as the package now supports subpath imports, and the changes are not breaking.