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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(esm): converted the package to esm #297

Merged
merged 10 commits into from
May 30, 2023
Merged

refactor(esm): converted the package to esm #297

merged 10 commits into from
May 30, 2023

Conversation

travi
Copy link
Member

@travi travi commented Dec 16, 2021

BREAKING CHANGE: @semantic-release/commit-analyzer is now a native ES Module. It has named exports
for each plugin hook (analyzeCommits)

fixes #296

BREAKING CHANGE: `@semantic-release/commit-analyzer` is now a native ES Module. It has named exports
for each plugin hook (`analyzeCommits`)

fixes #296
@travi
Copy link
Member Author

travi commented Dec 16, 2021

i've spent more time that i'd like to admit trying to understand several instances of this error:

 Error {
    code: 'ERR_REQUIRE_ESM',
    message: `require() of ES Module /Users/path/to/semantic-release/commit-analyzer/test/fixtures/release-rules.js from /Users/path/to/semantic-release/commit-analyzer/noop.js not supported.␊
    Instead change the require of release-rules.js in /Users/path/to/semantic-release/commit-analyzer/noop.js to a dynamic import() which is available in all CommonJS modules.`,
  }

and where noop.js was even coming from. in the end, i think it is related to sindresorhus/import-from#9 and

? importFrom.silent(__dirname, releaseRules) || importFrom(cwd, releaseRules)

we may need to consider an alternative approach with this. any thoughts come to mind @gr2m?

@gr2m
Copy link
Member

gr2m commented Dec 17, 2021

Not out of hand, I don't think I used importFrom in any other ESM project. Maybe we can use another approach that doesn't need import-from?

@travi
Copy link
Member Author

travi commented Dec 17, 2021

Maybe we can use another approach that doesn't need import-from?

yeah, i think thats what i'm looking for ideas around. i havent thought too far about it yet, but i think we may need to come up with alternative ideas for this piece.

@dgilperez
Copy link

May I ask for status of this? I just switched an App to ESM and I found semantic-release is not working, which probably depends on this PR to be merged (among others). Since I have this thing fresh, I may add some value to any roadblock, if you can provide some status for me.

@travi
Copy link
Member Author

travi commented Jun 6, 2022

I just switched an App to ESM and I found semantic-release is not working, which probably depends on this PR to be merged

semantic-release runs as its own executable, so the type of your project should have no impact on how semantic-release executes. if you have a .releaserc.js to configure semantic-release, that is the one exception and you may need to convert that file to match your project.

@dgilperez
Copy link

dgilperez commented Jun 6, 2022 via email

@sheerlox
Copy link
Member

i've spent more time that i'd like to admit trying to understand several instances of this error:

 Error {
    code: 'ERR_REQUIRE_ESM',
    message: `require() of ES Module /Users/path/to/semantic-release/commit-analyzer/test/fixtures/release-rules.js from /Users/path/to/semantic-release/commit-analyzer/noop.js not supported.␊
    Instead change the require of release-rules.js in /Users/path/to/semantic-release/commit-analyzer/noop.js to a dynamic import() which is available in all CommonJS modules.`,
  }

and where noop.js was even coming from. in the end, i think it is related to sindresorhus/import-from#9 and

? importFrom.silent(__dirname, releaseRules) || importFrom(cwd, releaseRules)

hey @travi 👋 I see the same code has been released in v11.0.0 of @semantic-release/release-notes-generator (about a month ago), so I'm wondering if this is still a blocking point, since versions of import-from are both set to ^4.0.0 and the importFrom function is used in the same exact way?

And if there's still an issue here, could that imply the same issue is present in v11 of @semantic-release/release-notes-generator?

@travi
Copy link
Member Author

travi commented May 26, 2023

to be honest, i haven't had the chance to revisit the conversion of this plugin, so i'm not sure if the things that we've learned from converting other packages would apply in a way that would get us past this concern in this context.

And if there's still an issue here, could that imply the same issue is present in v11 of @semantic-release/release-notes-generator?

are you encountering a problem in the release notes generator that you think could be related to this?

@sheerlox
Copy link
Member

are you encountering a problem in the release notes generator that you think could be related to this?

not at all! I came to this issue because I was trying to update some dependencies on my conventional-changelog preset (e.g. insurgent-lab/conventional-changelog-preset#8), then figured out that if I transitioned the preset to ESM, @semantic-release/commit-analyzer wouldn't be able to load it anymore, right?

this is not a big deal though, so I'll just monitor this PR and transition the preset to ESM when it becomes possible, no pressure 😉

@travi
Copy link
Member Author

travi commented May 26, 2023

then figured out that if I transitioned the preset to ESM, @semantic-release/commit-analyzer wouldn't be able to load it anymore, right?

this is very possible, and quite honestly likely. i dont have the full picture in my head at the moment, but from brief consideration this morning, i do believe you are correct. i think we are in an ok state with release-notes-generator because existing presets have not yet converted to esm.

you are highlighting that we are at risk of breakage for folks if a preset does convert since i didnt come across the consideration that i caught above in the conversion of the release-notes-generator. i really appreciate that you dug deep enough to figure this out while considering the conversion of your preset, but especially appreciate that you reached out with the information to have a conversation with us about it.

@travi
Copy link
Member Author

travi commented May 26, 2023

the other thing that i think is worth calling out as a result of this information, as it relates to moving the effort forward for this package, i think we could consider this detail to not be a blocker for converting this plugin. that choice would leave us in the situation where we would be unable to load presets that have been converted to esm, but we are already in that situation with release-notes-generator. while we may want to enable loading of esm presets, it could make sense to follow up with a separate effort for enabling that sometime after we convert this package to esm.

@sheerlox
Copy link
Member

sheerlox commented May 26, 2023

it's a pleasure really, been using semantic-release for more than 4 years, so many thanks to you guys for creating and maintaining this awesome set of tools!

P.S: I'd love to offer to contribute on this issue, but right now I already have a lot on my mind. I might come back to it in a while if it is still hanging 😉

@sheerlox
Copy link
Member

it could make sense to follow up with a separate effort for enabling that sometime after we convert this package to esm.

that definitely makes sense to me!

travi added 8 commits May 26, 2023 17:03
since we dont yet have a solution for loading from esm and are already limited to cjs in other areas

for #296
BREAKING CHANGE: the minimum required version of node is now v18
…e to v20.1.0

to ensure support for loading ESM plugins

BREAKING CHANGE: the minimum required version of semantic-release is now v20.1.0 in order to support
loading ESM plugins
@travi travi marked this pull request as ready for review May 30, 2023 02:27
@travi travi changed the base branch from master to beta May 30, 2023 02:32
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

w00p it's happening dot gif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ES Module
4 participants