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(node-resolve): bump is-builtin-module version, imports with a trailing slash #1424

Merged
merged 3 commits into from Apr 4, 2023

Conversation

rsweeneydev
Copy link
Contributor

Rollup Plugin Name: node-resolve

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:
#1211

Description

Adding a trailing slash to a module name in an import statement is the convention for telling node to use the local module with that name, rather than the builtin module with the same name.

For an example, see the docs for punycode which is available as both a builtin and standalone module for node.

The is-builtin-module package did not respect this convention. A fix was made and released in is-builtin-modules v3.2.1. This change bumps the version of the dependency and adds a test case to verify that import statements with a trailing slash are not treated as builtin modules by the node-resolve package.

@rsweeneydev rsweeneydev marked this pull request as ready for review January 31, 2023 15:11
@shellscape shellscape changed the title Bump the version of is-builtin-module to pick up the fix for imports with a trailing slash fix(node-resolve): bump is-builtin-module version, imports with a trailing slash Jan 31, 2023
Copy link
Collaborator

@shellscape shellscape left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the additional test!

@shellscape
Copy link
Collaborator

@rsweeney21 I was all set to merge this but a prior PR has updated our lockfile. Please rebase to upstream/master and run pnpm install at the repo root to resolve the conflict. We'll merge as soon as you're able to do that.

@shellscape
Copy link
Collaborator

I was actually able to resolve the conflict using the web editor.

@shellscape shellscape merged commit aa5a994 into rollup:master Apr 4, 2023
7 checks passed
@shellscape
Copy link
Collaborator

thanks!

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.

None yet

3 participants