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

Prepare release of CJS and Node Resolution updates #2129

Merged
merged 7 commits into from
Mar 11, 2023
Merged

Prepare release of CJS and Node Resolution updates #2129

merged 7 commits into from
Mar 11, 2023

Conversation

Westbrook
Copy link
Member

What I did

  1. Accepted @43081j's updates and pre-released them.
  2. Users are testing them in [@web/dev-server]: @rollup/plugin-commonjs version >18 breaks commonjs imports #1700 (comment)
  3. It's possible (though I'm likely lying to myself) that this support avoiding some of the failures in chore: update all rollup dependencies to run CI with #2114

43081j and others added 7 commits February 22, 2023 09:10
It is possible a rollup plug injects its own child plugins by hooking
into the rollup `options` hook and mutating the options to have extra
plugs.

This change introduces a check for such plugins and calls them before
the host plugin in case they successfully resolve a path first.

Similarly, the `resolve` method of plugins receives an options object to
hold state for the resolution plugins. We currently lose this object
when calling through to `resolveImport`, which can confuse third-party
plugins/resolvers heavily.

To fix this, `resolveImport` now also accepts a `resolverOptions` so we
can pass it on when calling child plugins.
@changeset-bot
Copy link

changeset-bot bot commented Feb 22, 2023

🦋 Changeset detected

Latest commit: d78e960

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@web/dev-server-core Minor
@web/dev-server-rollup Minor
@web/dev-server-esbuild Patch
@web/dev-server-hmr Patch
@web/dev-server-import-maps Patch
@web/dev-server-legacy Patch
@web/dev-server-storybook Patch
@web/dev-server Patch
@web/test-runner-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Westbrook
Copy link
Member Author

This seems to be working for people. Should we move ahead with merging it in? @43081j sound good?

@43081j
Copy link
Contributor

43081j commented Mar 11, 2023

I'm pretty confident it works 👍 so it's a go from me

Worst case I'll pick up any related issues and fix them. But don't think there will be any 🤞 We've been using it for a while now without any hiccups

@Westbrook Westbrook merged commit acc0a84 into master Mar 11, 2023
@Westbrook Westbrook deleted the next branch March 11, 2023 21:06
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