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

feat: support semantic-release higher than version 19 #235

Conversation

g-ongenae
Copy link

Fix #176.

Tested on @algoan/pubsub for publishing v6:

Copy link

@jackmoxley jackmoxley left a comment

Choose a reason for hiding this comment

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

Isn't removing the reload going to mean we are using the same semantic-release/npm each time?

@g-ongenae
Copy link
Author

@jackmoxley, That depends on how await import behaves, so I don't know for sure. If you have a way to test that, I'll gladly take a look into it.

@jhnns
Copy link

jhnns commented Nov 8, 2024

@g-ongenae Unfortunately await import is going to re-use the cached module entry. Node.js doesn't expose the internal module cache like require.cache.

@amanda-mitchell do you have any plans to make this work with ECMAScript modules? I've tried a solution with Worker threads (because each thread gets its own module cache), but that doesn't work because of function arguments that can't be cloned (probably context).

The safest solution I can think of is to actually copy the whole @semantic-release/npm folder to a temporary folder for each registry. Not very elegant, but it would probably work.

@amanda-mitchell
Copy link
Owner

I’ve been reading up on the internals of how node handles es modules, and I think that the cache can be deliberately suppressed by including a unique query string on each import.

I’ll try to find some time to play around with this soon.

@jhnns
Copy link

jhnns commented Nov 12, 2024

Oh wow, that is clever.

I think that's going to work for a single ES module. The problem is that you can only add a query string to your import, but not the following internal imports.

A possible solution could be to bundle the whole package in a single file upon install.

@jhnns
Copy link

jhnns commented Nov 12, 2024

Alternatively we could ask the maintainers of semantic-release to remove the state on module level (or send a PR). It's probably also more testable this way.

@amanda-mitchell
Copy link
Owner

I think that's going to work for a single ES module. The problem is that you can only add a query string to your import, but not the following internal imports.

True! I have not yet looked into where the module level state is stored in the latest version of the module: this will only work if it’s at the top.

Alternatively we could ask the maintainers of semantic-release to remove the state on module level (or send a PR)

That’s certainly the most attractive option! …although I also wouldn’t hold my breath for it, given the “you shouldn’t do that” response to the issue you opened.

@amanda-mitchell
Copy link
Owner

Update: I took a quick peek through semantic-release/npm this morning, and in the current code, it looks like the only module-level state is the name of a temp file that is created in the root module.

Knowing this, it should be able to orchestrate something that works for the current version.

@amanda-mitchell
Copy link
Owner

@jackmoxley, @jhnns, @g-ongenae, after significant struggle, I got all of it working in a fresh branch.

Thanks for the nudge to fix this!

@jhnns
Copy link

jhnns commented Nov 17, 2024

Awesome, thank you ❤️

I also noticed that they only have this temp filename on the module level. My only concern is that they will introduce state again as they don't know of our use case... but let's see.

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.

semantic release v20 support
4 participants