module: expose requestType to user hooks#1
Draft
privatenumber wants to merge 2 commits into
Draft
Conversation
Add `context.requestType` ('import' | 'require') to both resolve and load
hook contexts, in sync (`module.registerHooks`) and async (`module.register`)
modes. The value is 'require' for `require()` calls (including the
`require(esm)` bridge) and 'import' for `import` statements, dynamic
`import()`, and `import.meta.resolve()`.
The async load hook also now receives `context.conditions` (previously
absent), and conditions are computed from the request type so sync hooks
no longer see ESM defaults when the load is on behalf of a `require()`.
This lets loaders distinguish a `require()` from an `import` without
inspecting the call stack, and gives them the correct package-exports
conditions for the request.
Refs: nodejs#51327
Pre-submission cleanup based on reviewer-style anticipation: * Extract `toRequestTypeString(requestType)` in esm/utils.js so the `kRequireInImportedCJS` -> `'require'` mapping lives in one place instead of being inlined at three call sites. * Extract `getConditionsForRequestType(requestType, defaultEsmConditions)` in modules/helpers.js so the request-type -> conditions selection logic is shared between sync and async hooks. * In `AsyncLoaderHooksOnLoaderHookWorker.load`, construct a fresh context object rather than mutating the one delivered over IPC. This matches the pattern used by the sibling `resolve` method and avoids surprising callers that pass a context through `nextLoad`. * Add tests covering the kImportInRequiredESM case (import inside a require()'d ESM stays `requestType: 'import'`), `import.meta.resolve()`, chained hooks where an intermediate hook drops the context, and the async-loader CJS bridge regression (the original repro for nodejs#51327). * Document `context.requestType` and the new `context.conditions` on the async load hook with YAML `changes:` entries, and cross-link `module.register` to its replacement `module.registerHooks`. Refs: nodejs#51327
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User-supplied module hooks cannot reliably tell whether the current resolve or load is on behalf of a
require()call or animport. The async load hook also receives nocontext.conditionsat all. Loaders (tsx, ts-node, swc-node) work around this today by inspectingError().stackfor internal frames likeloadAndTranslateForRequireInImportedCJS, which is brittle and breaks under composed loaders such as Yarn PnP.This patch:
context.requestType: 'import' | 'require'to bothresolveandloadhook contexts, in sync (module.registerHooks) and async (module.register) modes.'require'coversrequire(), therequire(esm)bridge, andrequire.resolve()in imported CJS;'import'coversimportstatements,import()expressions,import.meta.resolve(), and thekImportInRequiredESMcase.context.conditionsfromrequestType. Sync hooks for require-bridge loads now see CJS conditions instead of ESM defaults; async hooks receiveconditionsfor the first time.toRequestTypeString()inesm/utils.jsandgetConditionsForRequestType()inmodules/helpers.jsso the mapping lives in one place.AsyncLoaderHooksOnLoaderHookWorker.loadinstead of mutating the IPC-arrived one, matching the siblingresolve.customization_hooks.js:336.The field is added to both
module.registerHooksand the runtime-deprecatedmodule.registerso today'smodule.registerusers have parity while they migrate. The docs and the stability banner steer new users towardmodule.registerHooks.Refs: nodejs#51327