fix: properly hook submodule package exports#215
Merged
AbhiPrasad merged 2 commits intonodejs:mainfrom Dec 18, 2025
Merged
Conversation
bd85297 to
84840b8
Compare
0100387 to
36866ee
Compare
This requires passing the specifier down the chain to the hook function, in order to apply the hook function when it matches. By the time the hook is being matched, the submodule specifier is already overwritten to a full `file://` URL, and appears as if the user is requesting an arbitrary internal within the package by absolute path. Since the module detected in that path does not match any of the module specifiers, the hook would previously be skipped unless the `internals: true` option was set, _and_ the submodule export path matched the file subpath exactly. With the original specifier present, the hook can be applied, by matching the specifier requested by the user to the submodule specifier provided at hook creation. This is far less brittle as a hook author than using `internals: true`, because it does not depend on the internal structure of the package, but rather only the public API surface, which is much less likely to change.
Bring test:e2e back to passing on node 23 and above, by recognizing that a 'module.exports' export is now assigned as an alias for 'default'.
36866ee to
f431fef
Compare
timfish
approved these changes
Dec 18, 2025
AbhiPrasad
approved these changes
Dec 18, 2025
jsumners-nr
approved these changes
Dec 18, 2025
Merged
Contributor
|
This went out in v2.0.1! |
Member
|
We did not investigate what changed, but this does break dd-trace vitest instrumentation. So while we might do something that is out of the norm (which I can not determine right now), it does break things and I wonder about the semverness. |
7 tasks
Contributor
Author
|
@BridgeAR Do you have an example I could use to reproduce this? Even if you're doing something out of the norm, it seems like this shouldn't have been able to break anything that worked before, unless you were relying on not instrumenting a package submodule that was being explicitly requested to be instrumented. |
isaacs
added a commit
to isaacs/import-in-the-middle
that referenced
this pull request
Jan 8, 2026
This reverts commit a20f47a.
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
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.
This requires passing the specifier down the chain to the hook function,
in order to apply the hook function when it matches.
By the time the hook is being matched, the submodule specifier is
already overwritten to a full
file://URL, and appears as if the useris requesting an arbitrary internal within the package by absolute path.
Since the module detected in that path does not match any of the module
specifiers, the hook would previously be skipped unless the
internals: trueoption was set, and the submodule export path matched the filesubpath exactly.
With the original specifier present, the hook can be applied, by
matching the specifier requested by the user to the submodule specifier
provided at hook creation.
This is far less brittle as a hook author than using
internals: true,because it does not depend on the internal structure of the package, but
rather only the public API surface, which is much less likely to change.
Also: fix the test:e2e test. Turns out
exports['module.exports']is an alias for the default export on CJS modules when imported to ESM, as of node 23.