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

Allow importing using an isolated cache #40594

Closed
dead-claudia opened this issue Oct 25, 2021 · 8 comments
Closed

Allow importing using an isolated cache #40594

dead-claudia opened this issue Oct 25, 2021 · 8 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. stale

Comments

@dead-claudia
Copy link

dead-claudia commented Oct 25, 2021

Is your feature request related to a problem? Please describe.

  1. I would like to create a mechanism for hot reloading
  2. I would like to be able to execute a module in an isolated context for testing
  3. I would like to be able to start a script using a custom loader (ex: Hashbang not resolving --loader relative to entry #23868)

Describe the solution you'd like
Introduce a vm.importInNewContext(pathToModule, context, options?), where loader instances can be passed via options.loaders and options.aliases can be set to explicitly alias certain paths. The idea is that this will not use the parent import cache or require cache, but its own caches, and that it'll use the loaders specified above for that functionality. Once the reference to this module is lost, if no state external to the module is referenced (like TCP ports or whatnot or passed-in globals), the context should be made eligible for garbage collection.

Describe alternatives you've considered

  • Using child processes with appropriate --loader flags is possible assuming exports are async, but the intermediate layer will be error-prone, and it'll be too slow to use in performance-critical production servers, and is a complete non-starter for testing synchronous functions that act at least in part by observable side effect.
  • I could use the vm modules API, but there's a number of caveats that, while not blocking me entirely, necessarily make it very complicated and brittle:
    • I'd have to reimplement Node's resolution algorithm in its entirety, complete with loaders (if I wanted to support those)
    • If I wanted to ensure modules kept a clean, fully isolated context, I'd also have to both proxy all the various functions and reimplement module.syncBuiltinESMExports(). This would of course significantly slow down load times (a major problem for larger dependencies and applications) as it'd all be implemented in userland.
    • If I wanted to support module.createRequire, I'd have to use a wrapper around that mucks with the CJS module loader's internals somewhat (particularly around Module._cache, Module._initPaths, and friends) just to ensure the cache and resolved paths remain correctly separated from the main tree. (This is very obviously not meant to be publicly supported, and I doubt you'd want to support such a pattern, even though it's possible in theory to do.)
  • https://www.npmjs.com/package/proxyquire and https://www.npmjs.com/package/clear-module could be used together to mostly resolve the issue for CJS modules, but they of course obviously do not work for ESM modules, and they don't actually isolate the graph, only mutate it and change how it resolves.

Note: #36351 will also impact the dynamic import side of this.

@dead-claudia
Copy link
Author

Also, by side effect, this removes the need for the --loader option - the entry script could just import the loaders it needs and go from there.

@VoltrexKeyva VoltrexKeyva added the feature request Issues that request new features to be added to Node.js. label Oct 25, 2021
@GeoffreyBooth GeoffreyBooth added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. labels Oct 25, 2021
@GeoffreyBooth
Copy link
Member

cc @nodejs/loaders @nodejs/modules

@guybedford
Copy link
Contributor

guybedford commented Oct 26, 2021

My main concern here is always whether we should pre-emptively ship a Node.js specific API here, or if we should wait for / engage with TC39 in developing Realms and / or Compartments to a place where they can solve these use cases. //cc @kriskowal

If effort is being put into it, it may be worth considering putting that effort into standards as well as I do think it's a shame when we have so much knowledge at this point to end up with multiple implementations.

In whatever form I do encourage work here though as we need solutions of course.

@dead-claudia
Copy link
Author

Yeah, I was initially undecided myself @guybedford on whether to reach out here or TC39 (I saw that work myself, too), but I felt I'd get better attention here at least for my particular use cases.

@bmeck
Copy link
Member

bmeck commented Oct 26, 2021

I'd prefer to defer this to TC39 as well. In most cases you don't want a fully isolated cache and want to link across caches and there has been quite a bit of work on this (years) already for Compartments in various forms or fashions. I'm also hesitant to introduce an API that conflicts with --loader efforts by using first class values as the means of intercepting rather than a URL to the instrumentation mechanism if we add yet another form of this with Compartments. I personally don't want to maintain 3 workflows. 2 Seems acceptable since 1st class values and OOP reflection aren't priorities for --loader and having another workflow that supports it seems fine. If the main goal is just HMR/Mocking perhaps we could look at shipping and/or refining import.meta.hot which is roughly just exposing the test in https://github.com/nodejs/node/pull/39240/files#diff-08a5d722c9df06780af1b452e4394133c678b11eee19530ef0bd871f4a8a2c2f .

@JakobJingleheimer
Copy link
Contributor

I concur with Bradley: It seems like those 2 TC39 proposals will address this or at least much of it, so I would rather not duplicate efforts solving problems already being solved.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 17, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. stale
Projects
Development

No branches or pull requests

6 participants