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

feat: Add Hook named export #88

Merged
merged 2 commits into from
May 31, 2024
Merged

feat: Add Hook named export #88

merged 2 commits into from
May 31, 2024

Conversation

timfish
Copy link
Contributor

@timfish timfish commented May 29, 2024

import-in-the-middle has some ESM interop issues.

From here:

Specifically, namespace imports are required by the ESM spec to be objects. IITM's top-level export is not an object, but a callable function

This causes issues with some bundlers. Obviously bundled imports will not go through import-in-the-middle but using it should not inhibit bundlers as iitm can still hook build-in modules.

This PR adds a Hook named export and changes one cjs and esm test to use that new export.

@AbhiPrasad
Copy link

should we also update the README to make the named export the recommended usage?

@bengl
Copy link
Member

bengl commented May 31, 2024

@timfish should there best test additions too? I.e. if something's exported, it should be tested that that interface works?

@timfish
Copy link
Contributor Author

timfish commented May 31, 2024

should there best test additions too? I.e. if something's exported, it should be tested that that interface works?

I modified both an ESM and CJS test to use this new export!

@bengl
Copy link
Member

bengl commented May 31, 2024

I modified both an ESM and CJS test to use this new export!

Oops! I should have noticed that. 😅

And I see that other test files retain the usage of the default export. SGTM/LGTM.

@bengl bengl merged commit fb0e393 into nodejs:main May 31, 2024
45 checks passed
@timfish timfish deleted the fix/named-export branch May 31, 2024 13:07
@bengl bengl mentioned this pull request May 31, 2024
bengl added a commit that referenced this pull request May 31, 2024
$ git log --oneline --no-decorate v1.7.4..HEAD
9269d1f 1.8.0
74124e9 fix: handling of default and star exports (#85)
7fa4e9c Fix resolution of lib/register.js when using multiple loaders
(#76)
e7f05ca add node 22 to test matrix (#84)
fb0e393 feat: Add `Hook` named export (#88)
bef32f2 chore: don't create a package-lock file (#67)
bf3a4fb fix: Resolve re-exports of external modules (#78)
0d9f351 fix: Handle cyclical reference to current file (#83)
1e05e4b test: skip static-import on > v21 (#81)
bengl pushed a commit that referenced this pull request Jun 14, 2024
In #88 I added a named `Hook` export but forgot to update the TypeScript
types!
@github-actions github-actions bot mentioned this pull request Jul 8, 2024
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 this pull request may close these issues.

4 participants