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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for ESM presets #537

Merged
merged 6 commits into from Nov 6, 2023

Conversation

sheerlox
Copy link
Member

@sheerlox sheerlox commented Oct 3, 2023

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 ESM 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

cc @travi 馃槈

@sheerlox
Copy link
Member Author

sheerlox commented Oct 3, 2023

@sheerlox sheerlox marked this pull request as draft October 3, 2023 17:54
@sheerlox sheerlox marked this pull request as ready for review October 12, 2023 01:38
test/load-parser-config.test.js Show resolved Hide resolved
test/load-parser-config.test.js Show resolved Hide resolved
@sheerlox
Copy link
Member Author

sheerlox commented Nov 5, 2023

Looks like the tests are broken on Windows. I'll investigate and come back to you when I have fixed the issue.

Thanks for the follow-up and enthusiasm on this! 馃帀

@sheerlox
Copy link
Member Author

sheerlox commented Nov 5, 2023

The dynamic import on Windows requires absolute paths to be an URL. This has been fixed in v1.0.3.

Both PRs have been rebased against master and upgraded to use import-from-esm@1.0.3 馃帀

Copy link
Member

@travi travi left a comment

Choose a reason for hiding this comment

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

thanks a bunch for working through these and for your patience for me to get a chance to review

@sheerlox
Copy link
Member Author

sheerlox commented Nov 6, 2023

happy to contribute!

@travi travi merged commit 9dc87e0 into semantic-release:master Nov 6, 2023
11 checks passed
Copy link

github-actions bot commented Nov 6, 2023

馃帀 This PR is included in version 11.1.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

@travi
Copy link
Member

travi commented Nov 6, 2023

happy to contribute!

if you'd like to help us out on a more official basis, i'd be happy to invite you to the org and grant you triage rights to start with. let me know if that is something you'd be interested in, but no pressure if you arent interested

@sheerlox
Copy link
Member Author

sheerlox commented Nov 6, 2023

if you'd like to help us out on a more official basis, i'd be happy to invite you to the org and grant you triage rights to start with. let me know if that is something you'd be interested in, but no pressure if you arent interested

Sure, I'll give it my best shot, thank you for the invite!
Note that I'm currently evolving toward Elixir & BEAM (for which I'm working on a semantic-release plugin 馃憖), but I'll keep maintaining TS/JS packages I'm involved with, and would greatly appreciate contributing more to semantic-release on my (rarifying) free time 馃槃

@sheerlox sheerlox deleted the feat/support-esm-presets branch November 6, 2023 02:14
@mcmiddle592
Copy link

@sheerlox @travi I just pulled this change into my project and am facing an issue in my Windows Jenkins environment while running "npm run semantic-release":

[semantic-release] An error occurred while running semantic-release: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
at new NodeError (node:internal/errors:405:5)
at throwIfUnsupportedURLScheme (node:internal/modules/esm/load:136:11)
at defaultLoad (node:internal/modules/esm/load:87:3)
at nextLoad (node:internal/modules/esm/loader:163:28)
at ESMLoader.load (node:internal/modules/esm/loader:603:26)
at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
at new ModuleJob (node:internal/modules/esm/module_job:64:26)
at #createModuleJob (node:internal/modules/esm/loader:480:17)
at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34) {
code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME'
}
Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
at new NodeError (node:internal/errors:405:5)
at throwIfUnsupportedURLScheme (node:internal/modules/esm/load:136:11)
at defaultLoad (node:internal/modules/esm/load:87:3)
at nextLoad (node:internal/modules/esm/loader:163:28)
at ESMLoader.load (node:internal/modules/esm/loader:603:26)
at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
at new ModuleJob (node:internal/modules/esm/module_job:64:26)
at #createModuleJob (node:internal/modules/esm/loader:480:17)
at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34) {
code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME'
}04:49:55.031000 common.go:40: exit status 1

I wasn't expecting a breaking change, is there something that needs to be done in my project to handle this?

@sheerlox
Copy link
Member Author

sheerlox commented Nov 7, 2023

Hello @mcmiddle592, I'm sorry you're encountering an error because of that PR.

Could you please open an issue and share more details about the configuration you're using so I can better understand where the issue is coming from?

Note: I would expect import-from-esm to be mentioned in the stack trace, but that's not the case. Maybe it just isn't long enough.

@sheerlox
Copy link
Member Author

sheerlox commented Nov 7, 2023

That's definitely an issue with import-from-esm not covering the case where a Windows user could pass an absolute path. Please open an issue there instead to track this @mcmiddle592, but please know I'm already working on a fix.

@sheerlox
Copy link
Member Author

sheerlox commented Nov 7, 2023

@mcmiddle592 the fix has been released, please run npm update import-from-esm to install the fix and update your package-lock.json. My apologies for the inconvenience.

@travi
Copy link
Member

travi commented Nov 7, 2023

Since semantic-release/semantic-release#3037 was also released on Friday, I'm curious if that change is involved as well.

@sheerlox
Copy link
Member Author

sheerlox commented Nov 8, 2023

Yeah, that's definitely possible since resolve-from does not return an URL.
One workaround could be adding JSON support in import-from-esm so it could be used directly in place of resolve-from.

@dominykas
Copy link

dominykas commented Nov 8, 2023

Yes, it could be related, sorry - I do not have Windows to test this myself either.

Would the fix to prepend file:// be enough for this?

Then again - the only code paths that were changed were when the -e option is used - which is not the case here in the scripts? Would it make sense to pin to an earlier version of semantic-release (v22.0.6, I guess) to double check?

@sheerlox
Copy link
Member Author

sheerlox commented Nov 8, 2023

Would the fix to prepend file:// be enough for this?

That should solve the issue but feels a bit hackier than directly using import-from-esm, which would have the benefits of delegating resolution responsibility to it and unifying how we load configuration files (whether presets or extends). I'm planning to release JSON support in a few hours.

Then again - the only code paths that were changed were when the -e option is used - which is not the case here in the scripts? Would it make sense to pin to an earlier version of semantic-release (v22.0.6, I guess) to double check?

I'm not sure I understand what you mean, but the issue seems to arise because the user specifies an absolute path in its "extends" configuration/CLI option.

EDIT: JSON modules support released in v1.2.0

@mcmiddle592
Copy link

@sheerlox @travi thanks for all the help here and the quick responses. I pinned my Semantic-Release in the short term to 22.0.6 to workaround this issue. I won't be able to get to testing this until next week, but when I do I plan to ensure I am using the latest version of import-from-esm (instead of import-from) and see if my issue on Windows is resolved. If there is anything else I need to check then let me know.

@dominykas
Copy link

dominykas commented Nov 9, 2023

Sorry, I might not be following things in full - I responded because there was a change in semantic-release due to how it's loading the --extend configs, but it seems there was a similar change in @semantic-release/commit-analyzer, and the issue here is contained inside of @semantic-release/commit-analyzer? Or is there an actual problem with semantic-release itself? Or possibly both of course...

@sheerlox
Copy link
Member Author

sheerlox commented Nov 9, 2023

We think it might be both yes. As of now, the issue in @semantic-release/commit-analyzer has been fixed in release 1.1.3 of import-from-esm.

Since I added JSON module support yesterday, I just tried fixing the issue in semantic-release by using that library, which works well overall, except there's a test that tries to import a node_modules package containing a single index.json file (and no package.json). Currently, the library isn't able to handle that (and it is not specified in the NodeJS docs, although require seems to handle it correctly), but I think it wouldn't even work with an export field pointing to that file in package.json.

Unfortunately, I won't be available in the next few days so I won't be able to work on that issue. Maybe it would be worth prepending file:// in the meantime until I can tackle that issue in import-from-esm?

@travi
Copy link
Member

travi commented Nov 10, 2023

sorry for the slow response from me. the last couple days have been pretty busy for me. thank you for keeping the conversation moving forward.

since it sounds like any issue here is likely resolved, lets move the conversation about extending a config back to the other issue.

@mcmiddle592 we appreciate you remaining engaged and are interested in the results of your test with the import-from-esm update. as you can tell from the conversation, we suspect that there is also an update needed in core semantic-release for the other recent change related to esm loading. still interested in your results, but don't be surprised if you still encounter a problem until we make the other update.

@seebeen
Copy link
Member

seebeen commented Nov 10, 2023

Yes, it could be related, sorry - I do not have Windows to test this myself either.

@dominykas I'm on Windows. What needs doing? :)

@mcmiddle592
Copy link

@sheerlox @travi @dominykas just wanted to confirm that I did test replacing "import-from" with "import-from-esm" in my projects package-lock.json (by reinstalling my semantic release dependencies). This however did not resolve the issue (which I believe was expected), I still have the same problem:

[semantic-release] An error occurred while running semantic-release: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
at new NodeError (node:internal/errors:405:5)
at throwIfUnsupportedURLScheme (node:internal/modules/esm/load:136:11)
at defaultLoad (node:internal/modules/esm/load:87:3)
at nextLoad (node:internal/modules/esm/loader:163:28)
at ESMLoader.load (node:internal/modules/esm/loader:603:26)
at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
at new ModuleJob (node:internal/modules/esm/module_job:64:26)
at #createModuleJob (node:internal/modules/esm/loader:480:17)
at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34) {
code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME'
}
Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
at new NodeError (node:internal/errors:405:5)
at throwIfUnsupportedURLScheme (node:internal/modules/esm/load:136:11)
at defaultLoad (node:internal/modules/esm/load:87:3)
at nextLoad (node:internal/modules/esm/loader:163:28)
at ESMLoader.load (node:internal/modules/esm/loader:603:26)
at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
at new ModuleJob (node:internal/modules/esm/module_job:64:26)
at #createModuleJob (node:internal/modules/esm/loader:480:17)
at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34) {
code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME'
}10:48:07.392620 common.go:40: exit status 1

@sheerlox
Copy link
Member Author

Thanks for confirming the issue @mcmiddle592, we're now tracking this in semantic-release/semantic-release#3037. We'll let you know when the issue is resolved.

@kerasing
Copy link

@travi - hey this was a breaking change for me, because I was using require to access analyzeCommits to get the release type outside of semantic-release (in a github action i use to pre-emptively ensure the maintenance branches are within their range boundaries), and this shifted to requiring import.

@sheerlox
Copy link
Member Author

sheerlox commented Apr 7, 2024

hey this was a breaking change for me, because I was using require to access analyzeCommits to get the release type outside of semantic-release (in a github action i use to pre-emptively ensure the maintenance branches are within their range boundaries), and this shifted to requiring import.

Hi @kerasing, as you can see on this line of the PR diff, the function signature of analyzeCommits hasn't changed. Also, this package was already an ESM module before this PR.

The package was converted to ESM in v10 on June 2nd, 2023.

Is there something I'm missing about my PR changes that caused you to run into an issue?

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.

None yet

6 participants