Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add support for ESM presets#537

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@sheerlox
Copy link
Member

@sheerloxsheerlox commentedOct 3, 2023
edited
Loading

This PR adds support for loading pure ESM presets. This was not previously possible becauseimport-from usescreateRequire under the hood.

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

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

Related

cc@travi 😉

@sheerlox
Copy link
MemberAuthor

@sheerloxsheerlox marked this pull request as draftOctober 3, 2023 17:54
@sheerloxsheerloxforce-pushed thefeat/support-esm-presets branch from37e290a tod88beb8CompareOctober 12, 2023 01:37
@sheerloxsheerlox marked this pull request as ready for reviewOctober 12, 2023 01:38
@sheerloxsheerloxforce-pushed thefeat/support-esm-presets branch fromd88beb8 to5f6ac30CompareOctober 21, 2023 09:16
@sheerlox
Copy link
MemberAuthor

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! 🎉

travi reacted with heart emoji

@sheerloxsheerloxforce-pushed thefeat/support-esm-presets branch from9ee34c4 to3d51397CompareNovember 5, 2023 23:36
@sheerlox
Copy link
MemberAuthor

The dynamicimport on Windowsrequires absolute paths to be anURL. This has been fixed inv1.0.3.

Both PRs have been rebased againstmaster and upgraded to useimport-from-esm@1.0.3 🎉

Copy link
Member

@travitravi 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 reacted with heart emoji
@sheerlox
Copy link
MemberAuthor

happy to contribute!

@travitravi merged commit9dc87e0 intosemantic-release:masterNov 6, 2023
@github-actions
Copy link

🎉 This PR is included in version 11.1.0 🎉

The release is available on:

Yoursemantic-release bot 📦🚀

sheerlox reacted with rocket emoji

@travi
Copy link
Member

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
MemberAuthor

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 asemantic-release plugin 👀), but I'll keep maintaining TS/JS packages I'm involved with, and would greatly appreciate contributing more tosemantic-release on my (rarifying) free time 😄

@sheerloxsheerlox deleted the feat/support-esm-presets branchNovember 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
MemberAuthor

sheerlox commentedNov 7, 2023
edited
Loading

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

Could you pleaseopen 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 expectimport-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
MemberAuthor

sheerlox commentedNov 7, 2023
edited
Loading

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

@sheerlox
Copy link
MemberAuthor

@mcmiddle592 the fixhas been released, please runnpm update import-from-esm to install the fix and update yourpackage-lock.json. My apologies for the inconvenience.

mcmiddle592 reacted with thumbs up emoji

@travi
Copy link
Member

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

@sheerlox
Copy link
MemberAuthor

Yeah, that's definitely possible sinceresolve-fromdoes not return an URL.
One workaround could be adding JSON support inimport-from-esm so it could be used directly in place ofresolve-from.

@dominykas
Copy link

dominykas commentedNov 8, 2023
edited
Loading

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

Would the fix to prependfile:// 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 thescripts? Would it make sense to pin to an earlier version ofsemantic-release (v22.0.6, I guess) to double check?

@sheerlox
Copy link
MemberAuthor

sheerlox commentedNov 8, 2023
edited
Loading

Would the fix to prependfile:// be enough for this?

That should solve the issue but feels a bit hackier than directly usingimport-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 thescripts? Would it make sense to pin to an earlier version ofsemantic-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 inv1.2.0

mcmiddle592 reacted with thumbs up emoji

@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.

sheerlox reacted with thumbs up emoji

@dominykas
Copy link

dominykas commentedNov 9, 2023
edited
Loading

Sorry, I might not be following things in full - I responded because there was a change insemantic-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 withsemantic-release itself? Or possibly both of course...

@sheerlox
Copy link
MemberAuthor

sheerlox commentedNov 9, 2023
edited
Loading

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

Since I added JSON module support yesterday, I just triedfixing the issue insemantic-release by using that library, which works well overall, except there's a test that tries to import anode_modules package containing a singleindex.json file (and nopackage.json). Currently, the library isn't able to handle that (andit is not specified in the NodeJS docs, althoughrequire seems to handle it correctly), but I think it wouldn't even work with an export field pointing to that file inpackage.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 prependingfile:// in the meantime until I can tackle that issue inimport-from-esm?

@travi
Copy link
Member

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 commentedNov 10, 2023
edited
Loading

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 reacted with thumbs up emoji

@sheerlox
Copy link
MemberAuthor

Thanks for confirming the issue@mcmiddle592, we're now tracking this insemantic-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 usingrequire to accessanalyzeCommits 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 requiringimport.

@sheerlox
Copy link
MemberAuthor

hey this was a breaking change for me, because I was usingrequire to accessanalyzeCommits 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 requiringimport.

Hi@kerasing, as you can see onthis line of the PR diff, the function signature ofanalyzeCommits hasn't changed. Also, this packagewas already an ESM module before this PR.

The packagewas 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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@travitravitravi approved these changes

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sheerlox@travi@mcmiddle592@dominykas@seebeen@kerasing

[8]ページ先頭

©2009-2025 Movatter.jp