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

fix: CommonJs bare specifier resolution #96

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

timfish
Copy link
Contributor

@timfish timfish commented Jun 4, 2024

Closes #95

I've also made the getExports functions return Set<string> instead of string[] as this saves a load of unnecessary allocations.

I've also run this through the additional module tests in #93 and get the same results.

Copy link

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable to me!

lib/get-exports.js Outdated Show resolved Hide resolved
lib/get-exports.js Outdated Show resolved Hide resolved
@bengl bengl merged commit 29f51f6 into nodejs:main Jun 14, 2024
48 checks passed
@timfish timfish deleted the fix/cjs-resolution branch June 15, 2024 11:49
@tlhunter tlhunter mentioned this pull request Jun 18, 2024
tlhunter added a commit that referenced this pull request Jun 18, 2024
$ git log --oneline --no-decorate v1.8.0..HEAD
a3497a9 v1.8.1
88605a7 fix: Fallback to `parentLoad` if parsing fails (#104)
1c6f7b0 fix: Explicitly named exports should be exported over `export *` (#103)
29f51f6 fix: CommonJs bare specifier resolution (#96)
a8b39f7 fix: `parentResolve` is not a function (#100)
c87090d fix: Named export `Hook` missing from types (#92)
@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.

getFullCjsExports does not resolve re-exports from external dependencies
3 participants