Skip to content

fix(node-resolve): pass on isEntry flag and custom options#1016

Merged
shellscape merged 1 commit intomasterfrom
fix/node-resolve-isentry
Oct 19, 2021
Merged

fix(node-resolve): pass on isEntry flag and custom options#1016
shellscape merged 1 commit intomasterfrom
fix/node-resolve-isentry

Conversation

@lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Oct 8, 2021

Rollup Plugin Name: node-resolve

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

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

Event though I raised the rollup version used in tests, the plugin will work nicely with older Rollup versions that do not support the isEntry flag. In this case, isEntry: undefined will be passed on as an option to this.resolve, which in turn will be ignored when the flag is not supported.

List any relevant issue numbers:
rollup/rollup#4230

Description

This makes sure the plugin correctly passes on the isEntry flag introduced in recent Rollup versions to the this.resolve context function as well as the custom option. This will allow plugins to rely on these to e.g. resolve entry points to proxy files or do any other conditional resolving.

As noted above, this is purely a bugfix and should have no adverse effect when used with older Rollup versions.

Copy link
Member

@tjenkinson tjenkinson left a comment

Choose a reason for hiding this comment

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

Lgtm. Just spotted a typo

@lukastaegert lukastaegert force-pushed the fix/node-resolve-isentry branch 2 times, most recently from 3ff83fc to 27aa4e5 Compare October 8, 2021 08:40
@lukastaegert
Copy link
Member Author

Thanks. Also fixed the package file so that only the devDependencies version of Rollup changes, but not the peerDependencies version.

@lukastaegert lukastaegert marked this pull request as draft October 8, 2021 09:51
@lukastaegert
Copy link
Member Author

Switching to "draft" as after some thought, I think I should also forward the "custom" option to "this.resolve". Will convert it back once I added this.

@lukastaegert lukastaegert force-pushed the fix/node-resolve-isentry branch from 27aa4e5 to ed5b0ef Compare October 8, 2021 11:05
@lukastaegert lukastaegert marked this pull request as ready for review October 8, 2021 11:06
@lukastaegert lukastaegert changed the title fix(node-resolve): pass on isEntry flag fix(node-resolve): pass on isEntry flag and custom options Oct 8, 2021
@lukastaegert
Copy link
Member Author

Done and again ready for review.

Copy link
Member

@tjenkinson tjenkinson left a comment

Choose a reason for hiding this comment

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

👍

@lukastaegert lukastaegert force-pushed the fix/node-resolve-isentry branch from ed5b0ef to ab1de1f Compare October 8, 2021 12:19
@shellscape shellscape merged commit 12e1fee into master Oct 19, 2021
@shellscape shellscape deleted the fix/node-resolve-isentry branch October 19, 2021 13:45
}

const resolved = await doResolveId(this, importee, importer, opts);
const resolved = await doResolveId(this, importee, importer, resolveOptions.custom);

Choose a reason for hiding this comment

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

This commit broke plugin-node-resolve, see: #1023

Copy link
Member

Choose a reason for hiding this comment

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

opened #1029

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