test(remix-dev/vite): add Vanilla Extract to CSS tests #8000
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| import { test, expect } from "@playwright/test"; | ||
| import fs from "node:fs/promises"; | ||
| import path from "node:path"; | ||
| import url from "node:url"; | ||
|
|
||
| import { | ||
| createAppFixture, | ||
|
|
@@ -9,26 +12,44 @@ import { | |
| import type { Fixture, AppFixture } from "./helpers/create-fixture.js"; | ||
| import { PlaywrightFixture } from "./helpers/playwright-fixture.js"; | ||
|
|
||
| const __dirname = url.fileURLToPath(new URL(".", import.meta.url)); | ||
|
|
||
| const TEST_PADDING_VALUE = "20px"; | ||
|
|
||
| test.describe("Vite CSS build", () => { | ||
| let fixture: Fixture; | ||
| let appFixture: AppFixture; | ||
|
|
||
| test.beforeAll(async () => { | ||
| // set sideEffects for global vanilla extract css | ||
| let packageJson = JSON.parse( | ||
| await fs.readFile( | ||
| path.resolve(__dirname, "helpers", "node-template", "package.json"), | ||
| "utf-8" | ||
| ) | ||
| ); | ||
| packageJson.sideEffects = ["*.css.ts"]; | ||
|
Comment on lines
+24
to
+31
There was a problem hiding this comment. I realized that currently remix's template has So, vanilla extract global style won't work out-of-the-box. |
||
|
|
||
| fixture = await createFixture({ | ||
| compiler: "vite", | ||
| files: { | ||
| "package.json": JSON.stringify(packageJson, null, 2), | ||
| "remix.config.js": js` | ||
| throw new Error("Remix should not access remix.config.js when using Vite"); | ||
| export default {}; | ||
| `, | ||
| "vite.config.ts": js` | ||
| import { defineConfig } from "vite"; | ||
| import { unstable_vitePlugin as remix } from "@remix-run/dev"; | ||
| import { vanillaExtractPlugin } from "@vanilla-extract/vite-plugin"; | ||
|
|
||
| export default defineConfig({ | ||
| plugins: [ | ||
| vanillaExtractPlugin({ | ||
| emitCssInSsr: true, | ||
|
There was a problem hiding this comment. It surprised me that this is needed. After digging into it a bit, it makes sense that it's needed for our current approach to inlining CSS in dev though, however it's not clear to me why this would be needed for the production build. I'm happy for this PR to go through as-is for now since it's still capturing the current state and proving that it works. It'd be a good idea to follow this up separately though. There was a problem hiding this comment. Thanks for pointing out. On vanilla extract plugin side, I saw it's setting some default behavior for known ssr frameworks, so maybe similar thing could be done for remix on their side. config.plugins.some((plugin) =>
[
'astro:build',
'solid-start-server',
'vite-plugin-qwik',
'vite-plugin-svelte',
].includes(plugin.name),
)There was a problem hiding this comment. I'm leaning towards enabling Adding Remix to this list would definitely be an easy non-breaking fix though. There was a problem hiding this comment. I've opened a PR: vanilla-extract-css/vanilla-extract#1239 There was a problem hiding this comment. Thanks for checking in with VE! I also experimented with toggling When // in build/index.js
// originally from app/styles-vanilla-local.css.ts
import { setFileScope, endFileScope } from "@vanilla-extract/css/fileScope";
import { style } from "@vanilla-extract/css";
setFileScope("app/styles-vanilla-local.css.ts", "remix-template-remix");
const index = style({
background: "lightblue",
padding: "20px"
});
endFileScope();
// originally from app/routes/_index.tsx
jsx("div", { "data-css-vanilla-local": true, className: index ... })I was thinking There was a problem hiding this comment. That's a fair misunderstanding. If we revisit the options for the plugin, your comment is worth considering. There was a problem hiding this comment. Tests have been updated to use the latest Vanilla Extract Vite plugin where |
||
| }), | ||
| remix(), | ||
| ], | ||
| }); | ||
| `, | ||
| "app/root.tsx": js` | ||
|
|
@@ -69,10 +90,28 @@ test.describe("Vite CSS build", () => { | |
| padding: ${TEST_PADDING_VALUE}; | ||
| } | ||
| `, | ||
| "app/routes/_index/styles-vanilla-global.css.ts": js` | ||
| import { globalStyle } from "@vanilla-extract/css"; | ||
|
|
||
| globalStyle(".index_vanilla_global", { | ||
| background: "lightgreen", | ||
| padding: "${TEST_PADDING_VALUE}", | ||
| }); | ||
| `, | ||
| "app/routes/_index/styles-vanilla-local.css.ts": js` | ||
| import { style } from "@vanilla-extract/css"; | ||
|
|
||
| export const index = style({ | ||
| background: "lightblue", | ||
| padding: "${TEST_PADDING_VALUE}", | ||
| }); | ||
| `, | ||
| "app/routes/_index/route.tsx": js` | ||
| import "./styles-bundled.css"; | ||
| import linkedStyles from "./styles-linked.css?url"; | ||
| import cssModulesStyles from "./styles.module.css"; | ||
| import "./styles-vanilla-global.css"; | ||
| import * as stylesVanillaLocal from "./styles-vanilla-local.css"; | ||
|
|
||
| export function links() { | ||
| return [{ rel: "stylesheet", href: linkedStyles }]; | ||
|
|
@@ -84,7 +123,11 @@ test.describe("Vite CSS build", () => { | |
| <div data-css-modules className={cssModulesStyles.index}> | ||
| <div data-css-linked className="index_linked"> | ||
| <div data-css-bundled className="index_bundled"> | ||
| <div data-css-vanilla-global className="index_vanilla_global"> | ||
| <div data-css-vanilla-local className={stylesVanillaLocal.index}> | ||
| <h2>CSS test</h2> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
@@ -117,5 +160,13 @@ test.describe("Vite CSS build", () => { | |
| "padding", | ||
| TEST_PADDING_VALUE | ||
| ); | ||
| await expect(page.locator("#index [data-css-vanilla-global]")).toHaveCSS( | ||
| "padding", | ||
| TEST_PADDING_VALUE | ||
| ); | ||
| await expect(page.locator("#index [data-css-vanilla-local]")).toHaveCSS( | ||
| "padding", | ||
| TEST_PADDING_VALUE | ||
| ); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(EDIT: this diff shows
postcss.config.mjs, but I put it back tojs. Also I'm not sure postcss is even actually loading.mjsfile...)Something puzzling is happening in this seemingly unrelated test.
After installing
@vanilla-extract/vite-plugininintegration/package.json, this test fails (not only on CI but it also fails locally on my PC).My hunch is that, since
@vanilla-extract/vite-pluginhas dependency onpostcss-load-config, the existence of this in the visiblenode_modulesmight be affecting some postcss's internal config resolution. Butpostcss-load-configv3 seems to have issue with esm postcss/postcss-cli#387 (comment).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put it back to
postcss.config.jsand it seems addingpostcss-load-configv4 to root package.json fixes this issue 4222ed9 though I'm not sure if I'm doing the right thing...Please let me know if it should be handled in a different way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great find. I opened a PR to fix this at the source vanilla-extract-css/vanilla-extract#1231
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I don't have much experience with postcss, so I was quite at lost.
I would be happy to test if the fix is released. Please feel free to let me know!
EDIT: I verified it works now without fiddling with postcss-load-config 08bb2ed. Thanks for the fix!