Fix: The problem getViteConfig() duplicates some config #12312
Merged
Conversation
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
🦋 Changeset detectedLatest commit: 13c0fc7 The changes in this PR will be included in the next version bump. 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 |
github-actions
bot
added
the
pkg: astro
koyopro
force-pushed
the
fix/get-vite-config
branch
from
b477b4f to
e362cdb
Compare
koyopro
force-pushed
the
fix/get-vite-config
branch
from
e362cdb to
c0b5899
Compare
koyopro
commented
Oct 26, 2024
Comment on lines
+27
to
+34
| describe("getViteConfig", () => { | ||
| it("Does not change the default config.", async () => { | ||
| const command = "serve"; | ||
| const mode = "test"; | ||
| const configFn = getViteConfig({}); | ||
| const config = await configFn({ command, mode }); | ||
| const resolvedConfig = await resolveConfig(config, command, mode); | ||
| assert.deepStrictEqual(resolvedConfig.resolve.conditions, ["astro"]); |
There was a problem hiding this comment.
Before the fix, the test fails as follows. (The contents of the array are duplicated)
1 task
ematipico
approved these changes
Oct 27, 2024
Co-authored-by: Emanuele Stoppa <[email protected]>
bluwy
approved these changes
Oct 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Changes
In the
configmethod of@astro/plugin-middleware, we modified it to return an empty object instead of the received opts itself.There was an issue where some configs were duplicated when using
getViteConfig().This was caused by
@astro/plugin-middlewarereturning the receivedoptsas the config's return value, resulting inoptsbeing merged with itself.https://vite.dev/guide/api-plugin.html#config
In the config, it is recommended to return only the parts you want to change, rather than the entire modified config.
Since @astro/plugin-middleware does not need to change the config itself, it seems correct to modify it to return an empty object as shown above.
Fixes: When using getViteConfig, the files listed in setupFiles are loaded twice. · Issue #12287 · withastro/astro
Testing
Before the fix, we tested that the return value of Vite's resolveConfig had a resolve.conditions array with duplicate elements like ['astro', 'astro']. After the change, the duplicates were resolved, resulting in ['astro'].
Docs
It is unlikely users rely on the existing buggy behavior, so updating the docs isn't necessary.