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

fix: merge client and ssr values for pluginContainer.getModuleInfo #18895

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Dec 5, 2024

Description

fix withastro/astro#12635

When calling server.pluginContainer.getModuleInfo(), if both client and ssr modules exists, the info is merged instead of only having the client module take precedence. This is done via proxy the same way as EnvironmentPluginContainer.getModuleInfo is implemented so we don't break the proxy behaviour.

This PR also fixes accessing (mixed) ModuleNode's info and meta properties to also merged them for client and ssr moudules instead of having the client module take precedence. This fix isn't needed for the linked issue above, but I did it anyways since I initially thought I needed the fix here.

@bluwy bluwy added the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Dec 5, 2024
Comment on lines +963 to +972
const clientModuleInfo = (
this.environments.client as DevEnvironment
).pluginContainer.getModuleInfo(id)
const ssrModuleInfo = (
this.environments.ssr as DevEnvironment
).pluginContainer.getModuleInfo(id)

if (clientModuleInfo == null && ssrModuleInfo == null) return null

return new Proxy({} as any, {
Copy link
Member Author

Choose a reason for hiding this comment

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

This code here and below looks similar to the change in mixedModuleGraph. I initially also tried to refactor so we pass the mixed module graph to the plugin container so we can share the code, but it turns out a little fragile I think especially type-wise.

We also need to explicitly call the getModuleInfo here to trigger lazily creating module.info in EnvironmentPluginContainer.

@patak-dev patak-dev merged commit 258cdd6 into main Dec 10, 2024
16 checks passed
@patak-dev patak-dev deleted the fix-info-meta branch December 10, 2024 09:53
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 21, 2024
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.1 | 6.0.5 |


## [v6.0.5](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small605-2024-12-20-small)

-   fix: esbuild regression (pin to 0.24.0) ([#19027](vitejs/vite#19027)) ([4359e0d](vitejs/vite@4359e0d)), closes [#19027](vitejs/vite#19027)


## [v6.0.4](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small604-2024-12-19-small)

-   fix: `this.resolve` skipSelf should not skip for different `id` or `import` ([#18903](vitejs/vite#18903)) ([4727320](vitejs/vite@4727320)), closes [#18903](vitejs/vite#18903)
-   fix: fallback terser to main thread when function options are used ([#18987](vitejs/vite#18987)) ([12b612d](vitejs/vite@12b612d)), closes [#18987](vitejs/vite#18987)
-   fix: merge client and ssr values for `pluginContainer.getModuleInfo` ([#18895](vitejs/vite#18895)) ([258cdd6](vitejs/vite@258cdd6)), closes [#18895](vitejs/vite#18895)
-   fix(css): escape double quotes in `url()` when lightningcss is used ([#18997](vitejs/vite#18997)) ([3734f80](vitejs/vite@3734f80)), closes [#18997](vitejs/vite#18997)
-   fix(css): root relative import in sass modern API on Windows ([#18945](vitejs/vite#18945)) ([c4b532c](vitejs/vite@c4b532c)), closes [#18945](vitejs/vite#18945)
-   fix(css): skip non css in custom sass importer ([#18970](vitejs/vite#18970)) ([21680bd](vitejs/vite@21680bd)), closes [#18970](vitejs/vite#18970)
-   fix(deps): update all non-major dependencies ([#18967](vitejs/vite#18967)) ([d88d000](vitejs/vite@d88d000)), closes [#18967](vitejs/vite#18967)
-   fix(deps): update all non-major dependencies ([#18996](vitejs/vite#18996)) ([2b4f115](vitejs/vite@2b4f115)), closes [#18996](vitejs/vite#18996)
-   fix(optimizer): keep NODE_ENV as-is when keepProcessEnv is `true` ([#18899](vitejs/vite#18899)) ([8a6bb4e](vitejs/vite@8a6bb4e)), closes [#18899](vitejs/vite#18899)
-   fix(ssr): recreate ssrCompatModuleRunner on restart ([#18973](vitejs/vite#18973)) ([7d6dd5d](vitejs/vite@7d6dd5d)), closes [#18973](vitejs/vite#18973)
-   chore: better validation error message for dts build ([#18948](vitejs/vite#18948)) ([63b82f1](vitejs/vite@63b82f1)), closes [#18948](vitejs/vite#18948)
-   chore(deps): update all non-major dependencies ([#18916](vitejs/vite#18916)) ([ef7a6a3](vitejs/vite@ef7a6a3)), closes [#18916](vitejs/vite#18916)
-   chore(deps): update dependency [@rollup/plugin-node-resolve](https://github.com/rollup/plugin-node-resolve) to v16 ([#18968](vitejs/vite#18968)) ([62fad6d](vitejs/vite@62fad6d)), closes [#18968](vitejs/vite#18968)
-   refactor: make internal invoke event to use the same interface with `handleInvoke` ([#18902](vitejs/vite#18902)) ([27f691b](vitejs/vite@27f691b)), closes [#18902](vitejs/vite#18902)
-   refactor: simplify manifest plugin code ([#18890](vitejs/vite#18890)) ([1bfe21b](vitejs/vite@1bfe21b)), closes [#18890](vitejs/vite#18890)
-   test: test `ModuleRunnerTransport` `invoke` API ([#18865](vitejs/vite#18865)) ([e5f5301](vitejs/vite@e5f5301)), closes [#18865](vitejs/vite#18865)
-   test: test output hash changes ([#18898](vitejs/vite#18898)) ([bfbb130](vitejs/vite@bfbb130)), closes [#18898](vitejs/vite#18898)


## [v6.0.3](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small603-2024-12-05-small)

-   fix: handle postcss load unhandled rejections ([#18886](vitejs/vite#18886)) ([d5fb653](vitejs/vite@d5fb653)), closes [#18886](vitejs/vite#18886)
-   fix: make handleInvoke interface compatible with invoke ([#18876](vitejs/vite#18876)) ([a1dd396](vitejs/vite@a1dd396)), closes [#18876](vitejs/vite#18876)
-   fix: make result interfaces for `ModuleRunnerTransport#invoke` more explicit ([#18851](vitejs/vite#18851)) ([a75fc31](vitejs/vite@a75fc31)), closes [#18851](vitejs/vite#18851)
-   fix: merge `environments.ssr.resolve` with root `ssr` config ([#18857](vitejs/vite#18857)) ([3104331](vitejs/vite@3104331)), closes [#18857](vitejs/vite#18857)
-   fix: no permission to create vite config file ([#18844](vitejs/vite#18844)) ([ff47778](vitejs/vite@ff47778)), closes [#18844](vitejs/vite#18844)
-   fix: remove CSS import in CJS correctly in some cases ([#18885](vitejs/vite#18885)) ([690a36f](vitejs/vite@690a36f)), closes [#18885](vitejs/vite#18885)
-   fix(config): bundle files referenced with imports field ([#18887](vitejs/vite#18887)) ([2b5926a](vitejs/vite@2b5926a)), closes [#18887](vitejs/vite#18887)
-   fix(config): make stacktrace path correct when sourcemap is enabled ([#18833](vitejs/vite#18833)) ([20fdf21](vitejs/vite@20fdf21)), closes [#18833](vitejs/vite#18833)
-   fix(css): rewrite url when image-set and url exist at the same time ([#18868](vitejs/vite#18868)) ([d59efd8](vitejs/vite@d59efd8)), closes [#18868](vitejs/vite#18868)
-   fix(deps): update all non-major dependencies ([#18853](vitejs/vite#18853)) ([5c02236](vitejs/vite@5c02236)), closes [#18853](vitejs/vite#18853)
-   fix(html): allow unexpected question mark in tag name ([#18852](vitejs/vite#18852)) ([1b54e50](vitejs/vite@1b54e50)), closes [#18852](vitejs/vite#18852)
-   fix(module-runner): decode uri for file url passed to import ([#18837](vitejs/vite#18837)) ([88e49aa](vitejs/vite@88e49aa)), closes [#18837](vitejs/vite#18837)
-   refactor: fix logic errors found by no-unnecessary-condition rule ([#18891](vitejs/vite#18891)) ([ea802f8](vitejs/vite@ea802f8)), closes [#18891](vitejs/vite#18891)
-   chore: fix duplicate attributes issue number in comment ([#18860](vitejs/vite#18860)) ([ffee618](vitejs/vite@ffee618)), closes [#18860](vitejs/vite#18860)


## [v6.0.2](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small602-2024-12-02-small)

-   chore: run typecheck in unit tests ([#18858](vitejs/vite#18858)) ([49f20bb](vitejs/vite@49f20bb)), closes [#18858](vitejs/vite#18858)
-   chore: update broken links in changelog ([#18802](vitejs/vite#18802)) ([cb754f8](vitejs/vite@cb754f8)), closes [#18802](vitejs/vite#18802)
-   chore: update broken links in changelog ([#18804](vitejs/vite#18804)) ([47ec49f](vitejs/vite@47ec49f)), closes [#18804](vitejs/vite#18804)
-   fix: don't store temporary vite config file in `node_modules` if deno ([#18823](vitejs/vite#18823)) ([a20267b](vitejs/vite@a20267b)), closes [#18823](vitejs/vite#18823)
-   fix(css): referencing aliased svg asset with lightningcss enabled errored ([#18819](vitejs/vite#18819)) ([ae68958](vitejs/vite@ae68958)), closes [#18819](vitejs/vite#18819)
-   fix(manifest): use `style.css` as a key for the style file for `cssCodesplit: false` ([#18820](vitejs/vite#18820)) ([ec51115](vitejs/vite@ec51115)), closes [#18820](vitejs/vite#18820)
-   fix(optimizer): resolve all promises when cancelled ([#18826](vitejs/vite#18826)) ([d6e6194](vitejs/vite@d6e6194)), closes [#18826](vitejs/vite#18826)
-   fix(resolve): don't set builtinModules to `external` by default ([#18821](vitejs/vite#18821)) ([2250ffa](vitejs/vite@2250ffa)), closes [#18821](vitejs/vite#18821)
-   fix(ssr): set `ssr.target: 'webworker'` defaults as fallback ([#18827](vitejs/vite#18827)) ([b39e696](vitejs/vite@b39e696)), closes [#18827](vitejs/vite#18827)
-   feat(css): format lightningcss error ([#18818](vitejs/vite#18818)) ([dac7992](vitejs/vite@dac7992)), closes [#18818](vitejs/vite#18818)
-   refactor: make properties of ResolvedServerOptions and ResolvedPreviewOptions required ([#18796](vitejs/vite#18796)) ([51a5569](vitejs/vite@51a5569)), closes [#18796](vitejs/vite#18796)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression with CSS importing from MDX files
3 participants