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

Support for user-land modules in the user-land snapshots #44277

Open
joyeecheung opened this issue Aug 18, 2022 · 1 comment
Open

Support for user-land modules in the user-land snapshots #44277

joyeecheung opened this issue Aug 18, 2022 · 1 comment
Labels
module Issues and PRs related to the module subsystem. snapshot Issues and PRs related to the startup snapshot

Comments

@joyeecheung
Copy link
Member

This issue is opened to discuss about supporting user-land modules in the user-land snapshots. Currently one has to use a bundler at snapshot building time if they want to include user-modules into the snapshot.

@joyeecheung
Copy link
Member Author

Copying from the thread in #44252:

goloveychuk commented 2 hours ago
just fyi, with this PR and small changes inside cjs/loader I've managed cjs require to work.
https://github.com/goloveychuk/node/commit/745bcef7b549b679667b9d26b83c00304ef58e0f (very dirty)
Good job!

joyeecheung commented 2 hours ago
@goloveychuk Thanks, that's good news. Though I've been thinking if we should only support loading user-modules at snapshot building time but disallowing that at runtime, and the other way around (what we have now), but not enabling module-loading at both snapshot building time and runtime, or it can be quite a challenge to keep the module lookup in-sync unless the user uses some kind of docker container to build and deploy things. But that's probably a topic separate from this PR.

goloveychuk commented 31 minutes ago
I think the answer will be different for different usecases.

For standalone distributed snapshot-script you would want to forbid requires in runtime because this probably means that you forgot it require inside snapshot.

For, let's say, aws lambda usage or for jest test runner you would want to allow both and use snapshots as "partially warmed up env".

I think best is to allow both at start, then collect a feedback and maybe, add some flags like "--require-snapshot-runtime-behaviour=warn,throw".
Also,this could accomplished in userland by patching Module._load

joyeecheung commented 6 minutes ago
@goloveychuk I think the partially warmed-up env is already possible if one bundles the code at snapshot building time. It's basically "you have to use a bundler if you want to be able to require at runtime, or you can load user-modules at snapshot building time if you don't need to load anything additional at runtime" vs "you can load user-land modules at both snapshot building time and runtime but the behavior would be weird if you do not guarantee that the environment is exactly the same". Compared to the quirky behaviors that the module loaders can have when the environment changes, the cost of an extra bundling step looks worthy to me. My plan is to enable the first and see if there is any use cases that can't be solved with building at runtime and if there is, consider refactoring the module loader to support more use cases - I am not sure which module loader we want to support first in that case, actually, because supporting CJS before ESM may send the wrong signal and meet pushbacks from other contributors who are advocating for deprecating CJS.

@joyeecheung joyeecheung added module Issues and PRs related to the module subsystem. snapshot Issues and PRs related to the startup snapshot labels Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. snapshot Issues and PRs related to the startup snapshot
Projects
None yet
Development

No branches or pull requests

1 participant