Fix Vite 8/Rolldown support and add cross-version compatibility coverage#13
Open
dino-careaccess wants to merge 1 commit intodavidmyersdev:mainfrom
Open
Fix Vite 8/Rolldown support and add cross-version compatibility coverage#13dino-careaccess wants to merge 1 commit intodavidmyersdev:mainfrom
dino-careaccess wants to merge 1 commit intodavidmyersdev:mainfrom
Conversation
Author
|
@davidmyersdev 👋 , I would appreciate your review! Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is more than a peer dependency bump.
The package was still using a
config()hook to injectbuild.rollupOptions.externalas a predicate function. That worked historically, but it breaks down in Vite 8/Rolldown when the consuming project also has its own staticrollupOptions.externalentries. In that case, Vite/Rolldown can end up with a mixed external config that includes both static entries and a function, which causes server/SSR-style builds to fail.The fix here is to stop mutating
rollupOptions.externaland instead externalize matching imports during resolution. That keeps the plugin compatible with Vite 8 while still preserving support for older Vite versions.What changed
resolveIdduring build instead of injectingrollupOptions.externalincludeexceptWhy the test coverage expanded
Once the peer range says Vite 8 is supported, this repo needs to prove that the implementation still works across the versions it claims to support.
This PR adds two layers of coverage:
Targeted integration tests around the actual Vite 8 regression:
rollupOptions.externalA compatibility matrix that exercises representative Vite releases from 2 through 8 in temporary fixtures
The matrix is important because the implementation change was not Vite-8-only. It replaced the core mechanism the plugin uses to externalize imports, so it was worth verifying that older Vite versions still behave correctly instead of assuming they do.
Verification
Verified locally with:
pnpm testpnpm buildpnpm test:matrixThe matrix covers:
Notes
rollupOptions.external = (id) => ...strategy, not just the peer range.