Repository navigation
RFC: Per Package Loader Hooks #18233
Description
Activity
- addedfeature requestIssues requesting new Node.js features.Issues requesting new Node.js features.esmIssues and PRs related to the ECMAScript Modules implementation.Issues and PRs related to the ECMAScript Modules implementation.
on Jan 18, 2018 It would be better to store all ESM info in: a single
"esm"property.
I suspect we will be adding several configs like this one inpackage.json, so I'd prefer it to be future-proof.@mcollina I've looked at allowing
requireto be instrumented as well but decided not to instrument it in loaders since it would need two separate hooks; one for synchronous operations, one for asynchronous. I'm fine putting it under a property / in an array / etc. but would prefer not to tie it specifically to ESM for now.Is the release of official ES module support certain to include this functionality? The particular mechanism used to interact with the module loading isn't important, just the ability to interact in the loading.
I'm looking into how one might load APM instrumentation into ES modules, and this level of support would be awesome (it is way better than the work arounds I've been able to come up with).
@lykkin APMs would probably use the existing APM example workflow and a global loader (
--loader) for nowWe spent some time experimenting with using loader hooks for instrumenting external modules, but weren't able to find a reasonable solution. Our test case involved a bare-bones express app (with express'
index.jsfile converted toindex.mjs), with a few different approaches:resolvehookWe were able to make this approach work using the APM loader example workflow as a base, but it is a fragile solution. Because what is ultimately returned from the
resolvehook is a fileurlandformat(not the module itself), we were forced to resolve to our own passthrough ESM file, which then actually imported the module (reaching up intonode_modules, because it's not a local dependency of the APM agent) and ultimately exported an instrumented version of it:import express from '../../../../express' // pointing back to node_modules import shimmer from '../../shimmer' import api from '../../../index' const { agent } = api export default shimmer.instrument(agent, express, 'express')
This approach also includes a potential problem with nested/duplicated dependencies. If two versions of a module are installed in the project, it is impossible to know which one was just loaded. Since the resolve hook depends on constructing a url from the specifier (in this case
'express'), any module with an internal dependency on the same module would resolve with the same url (assuming it's also an ES module, regardless of version. Working around this in the resolve hook would be very cumbersome.postLoadhookThis was an attempt to create an additional hook that would fire just after a module is instantiated, but before it is returned. The issue we ran into with this approach seemed to be that because all dependencies in a graph are instantiated in a single
this.module.instantiatecall, whenmodule.evaluateis executed (here), there are no usable references to which modules were loaded, and also nowhere to hook in during the operation because it all takes place inside V8.dynamicInstantiatehookWe also tried using a
dynamicInstantiatehook in combination with a modifiedresolve.resolvewould first check if.mjsinstrumentation existed for the given specifier (like the example loader), and then, if so, resolve withformat: 'dynamic'. The issue with this approach came from having to know the module'sexportsupfront, to use inexecute. This led to attemptingawait import(url)in order to assign the exported keys toexports, but that was unsuccessful, mainly due to the resulting recursion making keeping track of urls also overly cumbersome.
We're thinking what we need is either:
-
A new hook similar to
postLoad, which would intercept modules as they're instantiated by V8. You can see a prototype of the desired functionality here. -
A change to existing loader functionality that allows access to both the loaded module's export value and its original top-level specifier.
Again, the first approach with the
resolvehook works as described, but that pattern of hijacking the import process and explicitly pointing back up tonode_modulesis not the way to go. A better approach might be to utilizeimport.metawhen it's implemented, by storing the exports and specifier.
-
@lykkin as stated in the JS spec, the module namespace object should not be mutable. So, I'm not sure your idea with
postLoadwould actually be beneficial since the VM should prevent mutation of it directly.@lykkin we should setup a call probably before discussing adding hooks, since that might apply to both per package and global hooks, which are separate things.
@bmeck The
postLoadhook was just the simplest interface to get at the data required to instrument things like we currently do, I'm sure I'm stomping on some assumption I'm unaware of. Since the namespace object is immutable, theresolvehook proxying could work though there needs to be enough information provided to the proxy module to know what module it is proxying.A call sounds super helpful, I've been trying to read up on the discussion going on around this, though it sounds like there are a bunch of points I'm missing.
@lykkin very interesting feedback. Would you be interested in proposing a postLoad hook further here for Node?
@guybedford It sounds like there would have to be a case made upstream to v8 about exposing modules as they are being loaded, though there are also other interfaces that could be implemented to get the same functionality (e.g. an instrumentation API in v8 where you can register hooks on a module's methods and it will export the relevant data to a listener).
Having a hook in v8 for doing these kinds of things would be helpful for sure, but it sounds like I need to know more about v8's goals in this area to make a proper proposal. Would you know where I can read more about that?
@lykkin I wonder how far we might get with a translation approach something like:
instrument (moduleName: string) { return { exports: ['x', 'y', 'p'], instrument (exportName, value) { return wrap(exportName, value); } } }
export var x = 5; export const y= 10; export function p () { return ++x; } // this source is added by the instrumentation hook x = doInstrument(x); p = doInstrument(p); y = doInstrument(y);
The only problem with the above would be
constbindings, but perhaps a source replacement ofexport const->export letwould also be possible as a hack.At least using these hacks we could possibly explore the space somewhat. v8 API hooks would be the ideal certainly though, we'd just need to have a pretty good idea of what we'd want for Node and why before pushing I think?
(sorry @bmeck for going off-topic here, perhaps we should start a new thread)
Is this issue still relevant? There was a lot ongoing since this issue was opened.
@BridgeAR deferred while other things still are being looked into, but yes this is still being discussed though this proposal likely needs some updating
Reacted by Ruben Bridgewater@guybedford I want to re-re-revisit this.
@bmeck I'm still quite strongly against arbitrary hooks, so would prefer specific per-package features that are justified by direct use cases over arbitrary resolution hooking, and only arbitrary hooks where the limits of a more specific method are really being hit. The reason being that package invariants are much easier to handle with generic resolution rules that can be applied.
The general problem with using loaders due to it being a CLI flag and also being global and the persistent leaning on loaders to solve various use cases is recurring throughout multiple years. I think claiming there isn't a justification is a bit much at this point.
@bmeck okay, I'm not against declarative or implicit loaders at all. A loader configuration file or default file that turns loaders on is fine. What I'm against is such a file being possible to be enabled implicitly by a dependency in node_modules that affects resolution without the application developer being aware of it.
The general problem with using loaders due to it being a CLI flag and also being global and the persistent leaning on loaders to solve various use cases is recurring throughout multiple years.
I think there are ways to provide the ability to run loaders without needing CLI flags, without necessarily needing to scope loaders to packages. For example, we could support a
"nodeOptions"property onpackage.json, that would function the same way as the environment variableNODE_OPTIONSbut only be applied if thepackage.jsonin question was the nearest parentpackage.jsonof the entry point.But would this solve the problem? I think we need an issue clearly defining what these environments are that don’t support flags, and what other similar things they do and don’t support. Environment variables?
package.json? And then we can design to specifically address the problem, without painting an overbroad brush that lets people do unperformant things like ship untranspiled TypeScript packages with the expectation that a per-package loader will transpile them at runtime.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.
- addedstaleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.Issues and PRs marked stale due to inactivity and scheduled for automatic closure.
on May 23, 2022 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.
Per-package loader hooks
This proposal seeks strong support for per-package loader hooks.
This is a definition of how to achieve them.
Problem
Individual application and packages have loading considerations that vary. The ability to globally mutate the Node module system is problematic and causes packages to alter each other's behavior implicitly.
CommonJS had various abilities to mutate the CJS loader with
NODE_PATH,require.extentsion, etc. These have all been deprecated.Various workarounds to these use cases do exist but do not apply to all code using these patterns.
Example Use Cases
Proposal
Scope of hooks
Hooks must be confined to a well defined subsection of the URL space (
fs) used byimport.This proposal will define the boundaries of subsections to be:
Given the
fsof:Consumer and Author negotiation
In order to avoid recursive boundary crossing in one step, all paths will be resolved in two phases. This is similar to
Declaration of hooks
Per package loader hooks can be declared in a
package.jsonfile as a specifier to find using the globally defined resolution algorithm.Global hooks may affect this resolution, but package hooks may not.
This allows code coverage, instrumentation, etc. to access package hooks.
This also allows the hooks to exist outside of package boundaries. This file when loaded as a loader will be in a separate Module Map space from userland and only has the globally defined resolution algorithm.
Types of hooks
vm.Moduleto obtain a new URL if you need to create Module records dynamically.On the nature of static resolution
ESM is able to link statically and there should be a path to allow static / ahead of time usage of per package hooks ideally.
By only having a single
resolvehook, paths can be rewritten and observed to do in-source replacement.This is problematic however, since
vm.Modulelives in memory.Usage of such APIs on platforms without writable
fslike Heroku should have a path forward for these hooks.I recommend a combination of V8's SnapshotCreator when possible, and a flag to allow rewriting
vm.Modulereservations to a location on disk.Problem, multiple boundary crossing
If
entrywere toimport('../dep'). It would be handled in the typicalentryhooks thendephooks manner. This does not giveroota chance to intercept the imports.This is seen as a suitable limitation since
rootis presumed to have ownership ofentryanddep's source code by them existing within its directory. Edit theentryanddeppackages as needed in order to achieve hooking that goes throughroot's use cases.Composition
Hooks should have a means by which to achieve composition. This is needed for cases of multiple transformations. A package might seek to call a
superof sorts to get the result of a parent loader, and it may seek to do the exact opposite as a guard to ensure expected behavior.Loaders therefore need to have a concept of a parent loader hooks to defer to, or to ignore.
Changing hook allocation to be done using
newand providing the parent as a paremeter is sufficient for this:Example use cases for composition
Isolation
Hooks that are composed still are isolated by per-package boundaries. Nested packages will not fire the
parentloader hooks unless they cross into a package boundary with those hooks.Passing arbitrary data between instances can be problematic for both isolation and threading. Therefore the only data passed between instances of loaders will be transferables (including structured clone algorithm) or primitives.
The
parentpassed to the constructor of a loader will be a limited facade that only shows white listed properties and calls the relevant method on the true parent instance. It will ensure errors are thrown if given improper arguments length and/or non-transferable data.Per-package composition
Can be achieved by manually constructing the chain inside their per-package hook code.
Global composition
Can be achieved by providing multiple
--loaderflags. This allows for better debugging when development loaders need to be added. The full design of this is left to another RFC.Ignoring parents
In certain scenarios a package may need to ignore the parent loader. In those situations the hooks will be unable to defer to the default global behavior of the process, which may provide debugging behavior such as logging/code coverage/linting/etc.
For now escape hatches are punted on this design space to userland, but it is recommended that when using
NODE_ENV=developmentorNODE_ENV=testall loaders defer to the parent loader.Code signing invariant implications
Mutating the code loaded in a code signed bundle is problematic. Integrity checks of unexpectedly mutated imports should fail. This area needs more research. Use of any sort of in-source translation should be avoided.
Future research
Given the problems of ignoring scripts and code signing being unable to easily defer to parent loaders more design needs to be done around development workflows. Inspector tooling is the recommended approach. This may mean adding special hooks to inject loader hooks during development via a flag such as
--inspector-loader-hooks=LogImportthat may fire before per package hooks but ensures the inspector is running. Such hooks would not be suitable for production environments.This design also does not instrument CJS as loaders currently are not able to instrument CJS. It is not a design goal of this specification to add CJS support to ESM loaders; however, any design for CJS loaders that is presented should and will be considered for compatibility reasons.