Skip to content

fix(node-resolve): bump is-builtin-module version, imports with a trailing slash#1424

Merged
shellscape merged 3 commits intorollup:masterfrom
rsweeneydev:master
Apr 4, 2023
Merged

fix(node-resolve): bump is-builtin-module version, imports with a trailing slash#1424
shellscape merged 3 commits intorollup:masterfrom
rsweeneydev:master

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
@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.

3 participants