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

ModuleMap::clear_module_map should be rearchitectured #363

Closed
bartlomieju opened this issue Dec 1, 2023 · 0 comments · Fixed by #369
Closed

ModuleMap::clear_module_map should be rearchitectured #363

bartlomieju opened this issue Dec 1, 2023 · 0 comments · Fixed by #369
Assignees

Comments

@bartlomieju
Copy link
Member

Working on #263 we discovered that ModuleMap::clear_module_map is quite questionable way to interact with the runtime when snapshot is provided. Its purpose is to remove all modules from the module map but the ones passed as an argument. We use this method after we deserialize a snapshot, to remove all ext: modules and leave node: modules intact.

In #263 we need a way to load ext: modules during runtime, so as a temporary hack we added a few ext: modules to exclusions, but that's not a great solution. I think we need to rearchitect how module loading/resolution works, by always leaving all modules coming from the snapshot in the module map, but only allow importing ext: modules from other ext: modules (ie. user code should still not be able to load any of the ext: modules directly).

bartlomieju added a commit that referenced this issue Dec 6, 2023
…369)

A new "ModuleMap::resolve" function was added that is now used in
favor of directly calling "loader.resolve". This method performs
additional checks that prohibit resolution of "ext:" modules from 
modules that are not "ext:" or "node:" modules.

This will allow us to sunset "RuntimeOptions::preserve_snapshotted_modules"
and will greatly help with lazy loading ES modules for internal runtime code.

A bit different approach than suggested in the issue, but closes
#363.
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 a pull request may close this issue.

2 participants