-
Notifications
You must be signed in to change notification settings - Fork 404
fix: collect virtual module CSS in vite dev #2039
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
base: main
Are you sure you want to change the base?
Changes from all commits
d7de8ed
28e9822
983c75e
d1a23f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@solidjs/start": patch | ||
| --- | ||
|
|
||
| Fixed virtual module CSS not being collected in vite dev. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { Plugin } from "vite"; | ||
|
|
||
| const id = "virtual:virtualModule.css"; | ||
| const resolvedId = "\0" + id; | ||
|
|
||
| const virtualCSS = () => | ||
| ({ | ||
| name: "css-fixture-virtual-css", | ||
| resolveId(source) { | ||
| if (source === id) return resolvedId; | ||
| }, | ||
| load(id) { | ||
| if (id.startsWith(resolvedId)) | ||
| return `.virtualCss { background-color: var(--color-success); }`; | ||
| }, | ||
| }) satisfies Plugin; | ||
|
|
||
| export default virtualCSS; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| /** | ||
| * Patches the data-vite-dev-id attribute for style tags of virtual modules | ||
| * | ||
| * Per Vite's convention, virtual module ids are prefixed with \0 (null byte): | ||
| * https://vite.dev/guide/api-plugin#virtual-modules-convention | ||
| * | ||
| * However this null byte cannot be server rendered properly. | ||
| * Vite client runtime then fails to find style's with wrong null bytes, | ||
| * and instead inserts duplicate style's. | ||
| * | ||
| * This patch replaces the serializable /@id/__x00__ with the proper null byte, | ||
| * and has to run before Vite's client runtime: | ||
| * https://github.com/vitejs/vite/blob/130e7181a55c524383c63bbfb1749d0ff7185cad/packages/vite/src/client/client.ts#L529 | ||
| * | ||
| * TODO: This should be solved in Vite directly! | ||
| */ | ||
| const patch = function () { | ||
| document.querySelectorAll<HTMLElement>("style[data-vite-dev-id]").forEach(function (el) { | ||
| el.setAttribute("data-vite-dev-id", el.dataset.viteDevId!.replace("/@id/__x00__", "\0")); | ||
| }); | ||
| }; | ||
|
|
||
| const serializedPatch = `(${patch.toString()})();`; | ||
|
|
||
| const PatchVirtualDevStyles = (props: { nonce?: string }) => { | ||
| if (!import.meta.env.PROD) { | ||
| return <script nonce={props.nonce} innerHTML={serializedPatch} />; | ||
| } | ||
| }; | ||
|
|
||
| export default PatchVirtualDevStyles; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,40 +1,23 @@ | ||
| import path from "node:path"; | ||
| import { resolve } from "pathe"; | ||
| import type { DevEnvironment, EnvironmentModuleNode } from "vite"; | ||
|
|
||
| async function getViteModuleNode(vite: DevEnvironment, file: string) { | ||
| let nodePath = file; | ||
| let node = vite.moduleGraph.getModuleById(file); | ||
|
|
||
| if (!node) { | ||
| const resolvedId = await vite.pluginContainer.resolveId(file, undefined); | ||
| if (!resolvedId) return; | ||
|
|
||
| nodePath = resolvedId.id; | ||
| node = vite.moduleGraph.getModuleById(file); | ||
katywings marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (!node) { | ||
| nodePath = resolve(nodePath); | ||
| node = await vite.moduleGraph.getModuleByUrl(file); | ||
katywings marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (!node) { | ||
| await vite.moduleGraph.ensureEntryFromUrl(nodePath, false); | ||
| node = vite.moduleGraph.getModuleById(nodePath); | ||
| } | ||
|
|
||
| return node; | ||
| async function getViteModuleNode(vite: DevEnvironment, file: string, importer?: string) { | ||
| try { | ||
| const res = await vite.fetchModule(file, importer); | ||
| if (!("id" in res)) return; | ||
| return vite.moduleGraph.getModuleById(res.id); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. much cleaner! 🥇 but this is removing the fallback
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new line
So to "quickly" comment my thoughts about those: Removed ensureEntryFromUrlThe new Removed getModuleByUrl
Removed getModuleById
Extra words: To be completely honest, deleting all that old logic and replacing it with the simple two lines didn't exactly feel good, especially because we do not have lot's of tests around this. I tried my best to test the new logic, but there's certainly the chance that I missed something. So there is the risk that with the new logic some unexpected cases might pop up where CSS is not server rendered during the alpha. This however gives us the opportunity to then include those cases in the CSS fixture and bring back the few actually needed parts of the old logic, if necessary. What do you think of this plan @atilafassina (and @Brendonovich)?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for the explanation!! I agree with your reasoning: removing what looks wrong/unnecessary/redundant and watching for edge cases |
||
| } catch (err) {} | ||
| } | ||
|
|
||
| async function findModuleDependencies( | ||
| vite: DevEnvironment, | ||
| file: string, | ||
| deps: Set<EnvironmentModuleNode>, | ||
| crawledFiles = new Set<string>(), | ||
| importer?: string, | ||
| ) { | ||
| crawledFiles.add(file); | ||
| const module = await getViteModuleNode(vite, file); | ||
| const module = await getViteModuleNode(vite, file, importer); | ||
| if (!module?.id || deps.has(module)) return; | ||
|
|
||
| deps.add(module); | ||
|
|
@@ -53,7 +36,7 @@ async function findModuleDependencies( | |
| if (crawledFiles.has(dep)) { | ||
| continue; | ||
| } | ||
| await findModuleDependencies(vite, dep, deps, crawledFiles); | ||
| await findModuleDependencies(vite, dep, deps, crawledFiles, module.id); | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.