2 Commits

Author	SHA1	Message	Date
James Anderson	ce29f9c6aa	fix(scss): preprocess SCSS CSS-module composes deps via SassAwareFileSystemLoader (#1882) ... * fix(scss): preprocess SCSS CSS-module composes deps via SassAwareFileSystemLoader postcss-modules' built-in FileSystemLoader reads files referenced by `composes: className from './other.module.scss'` as raw text — without Sass preprocessing. For .scss files this leaves SCSS variables and bare @import directives in the CSS output, causing LightningCSS to crash with "Invalid empty selector" during production builds. Fix: introduce SassAwareFileSystemLoader that delegates to Vite's `preprocessCSS` API for every `composes:` dependency, so Sass compilation runs before CSS-module scoping. Register it via `css.modules.Loader` in vinext's Vite config hook and thread the resolved config through via a new `setSassLoaderResolvedConfig` call in the `configResolved` hook. Fixes the following deploy-suite tests (both sass and sass-embedded variants): - test/e2e/app-dir/scss/nm-module/nm-module.test.ts - test/e2e/app-dir/scss/nm-module-nested/nm-module-nested.test.ts - test/e2e/app-dir/scss/composes-external/composes-external.test.ts Part of #1825 (non-tilde CSS-module/composes cases; complements #1881) * fix(scss): address review feedback on SassAwareFileSystemLoader - Guard the css.modules.Loader injection: respect a user's explicit css.modules: false (mergeConfigRecursively would otherwise re-enable CSS Modules with an object override) and never clobber a user-provided custom Loader. The override is precomputed as a typed const so the conditional spread doesn't disturb the config literal's contextual typing. - Surface composes preprocessing failures via config.logger.warn instead of a bare catch {}, so missing composed classes are observable; a missing/unreadable dependency file now rejects like postcss-modules' FileSystemLoader did. - Use Vite's exported preprocessCSS + PreprocessCSSResult type via a top-level import instead of the lazy dynamic import and stringly typed casts. - Preserve parity for composes from non-*.module.* files: hand preprocessCSS a virtual *.module.* filename so plain '.css' deps are still scoped and export tokens extracted, matching the built-in FileSystemLoader. - Port the three failing Next.js fixtures (composes-external, nm-module, nm-module-nested) as production-build regression tests in tests/scss-composes.test.ts, plus a plain .module.css / non-module .css parity test. All three SCSS tests reproduce the original "[lightningcss minify] Invalid empty selector" crash when the Loader injection is disabled. - Add sass as a root devDependency so the SCSS tests (including the previously always-skipped tests/scss.test.ts) actually run in CI; drop the now-redundant knip ignore entry. Installing sass makes Vite's SassPreprocessorOptions resolve to the concrete sass types (previously any), so the tilde-importer's importers array now carries an explicit SassPreprocessorOptions["importers"] cast. * fix(scss): bind composes-loader config per plugin instance; scope extensionless deps - Replace the module-level resolved-config singleton with a per-vinext- instance binding (createSassAwareFileSystemLoader), so multiple builds in one process never preprocess composes deps with another build's config. - Cover extensionless composes deps in the virtual *.module.* rename (append .module.css) so their tokens are scoped instead of silently dropped; add a parity test case. - Pin url() pass-through behaviour for composed deps in the parity test (verbatim, never mangled into placeholders — matches the built-in FileSystemLoader, verified against a vanilla Vite build). * docs(scss): document the per-subtree recursion boundary of the composes loader Notes the two structural consequences of delegating nested composes recursion to preprocessCSS (per-subtree duplicate inlining collapsed by LightningCSS, and no cross-boundary cycle short-circuit for invalid circular composes), per review. * fix(scss): fail the build on composed-dep preprocessing errors Only a missing Sass implementation is downgraded to a logged warning; any other preprocessCSS error (e.g. a Sass syntax error in the composed dependency) now propagates and fails the build, matching postcss-modules' built-in FileSystemLoader instead of shipping a green build with silently-missing classes. Docstrings updated to accurately describe the silent empty-token path when no resolved config is bound. Adds a regression test for the fail-loudly path. * chore(deps): move sass to the workspace catalog; drop the now-dead dynamic skip in scss.test.ts Review feedback on #1882: sass belongs in the pnpm workspace catalog rather than pinned inline in the root package.json. With sass now a guaranteed root devDependency, the runtime describeIfSass skip in tests/scss.test.ts is dead code — the suite always runs — so make the unskip explicit by removing the conditional machinery. * rebuild lockfile	2026-06-11 00:29:51 +00:00
James Anderson	efa18a3f80	fix(pages-router): include _app module assets in served page HTML (#1232) ... * fix(pages-router): include _app module assets in served page HTML When a Pages Router page renders, vinext's prod server collects asset tags only for the matched page module. _app.tsx — which wraps every page — is omitted, so any CSS/JS it imports is built and emitted but never <link>ed from the rendered HTML. For SCSS users this is the LHF-5 cluster from the Next.js deploy suite: when global.scss is imported via _app.tsx, Vite compiles it correctly and emits assets/_app-*.css, but the browser never sees a <link rel="stylesheet"> for it. The page renders with the user-agent default colour (rgb(0, 0, 0)) instead of the SCSS-defined colour. Fix: embed the resolved pages/_app file path at build time and include it in the moduleIds passed to collectAssetTags, so the SSR manifest's CSS/JS entries for _app are emitted alongside the page's own. Includes a regression test (tests/scss.test.ts) plus a fixture page that imports a .scss file with a variable. The test covers App Router dev, App Router production build, Pages Router dev, and the Pages Router production server end-to-end (build → start prod server → fetch page → assert linked stylesheet has the resolved colour). Skipped automatically when sass is not installed — vinext relies on Vite's built-in SCSS preprocessor, which requires the user to install sass (or sass-embedded) themselves. Matches Vite's model. * fix: isolate SCSS fixture and silence TS check for optional sass import The first revision of this PR added an SCSS fixture under tests/fixtures/app-basic/app/nextjs-compat/scss-test/. That directory is scanned by every test that boots the shared app-basic fixture, so when sass is not installed (CI default) every page in app-basic fails the route-graph scan and 500s. jsx-in-js, app-css, node-modules-css, and ~dozens of integration tests went red as a result (CI run https://github.com/cloudflare/vinext/actions/runs/25913568939). The TypeScript check job also failed because `await import("sass")` produces TS2307 when sass isn't declared as a workspace dep. Fixes: - Drop the shared fixture under app-basic. tests/scss.test.ts now materialises its Pages Router SCSS fixture in a per-test tmpdir (same pattern as the existing Pages Router scenario), so the shared fixture's route-graph scan never sees a .scss import. No other test is affected. - The App Router test scenarios from the first revision are removed. They required additional node_modules wiring for @vitejs/plugin-rsc to resolve from a tmpdir and weren't needed to validate the fix — the actual change is in the Pages Router asset-collection path, and the Pages Router tests exercise it end-to-end (dev + production build + linked stylesheet assertion). - Replace @ts-expect-error with @ts-ignore on the dynamic sass import. sass may or may not be installed depending on the environment; when it is, there's no type error to expect and @ts-expect-error itself errors as unused. @ts-ignore is the right tool for an optional peer. The pages-server-entry.ts fix from the first revision is unchanged — this commit only re-shapes the test surface so it doesn't poison the shared fixture.	2026-05-15 12:15:54 +01:00