From 0a4b6979bffdab89c3d699671b6fc55972d527f4 Mon Sep 17 00:00:00 2001 From: Yinon Burgansky Date: Tue, 17 Mar 2026 00:58:22 +0200 Subject: [PATCH] fix: nitroV2Plugin compress public assets by default fixes #2096 - match `1.x` behavior for easier migration; can still be overridden to false for users who prefer uncompressed files. - extract common build test utilities. - fix tree shaking tests that were using unreliable regex (didn't account for minification and variable renaming); replace with a unique string identifier for better resilience, and add a complementary test for the tree shaking with side effects case. - add vitest `~` alias configuration, as the solid plugin wasn't resolving it in the test environment. --- .changeset/shiny-clowns-clap.md | 5 ++ .../compressed-public-assets.server.test.ts | 20 +++++++ .../routes/treeshaking/(no-side-effects).tsx | 2 +- .../server-secret-leak.server.test.ts | 40 +------------- .../src/routes/treeshaking/side-effects.tsx | 2 +- .../treeshaking/treeshake.server.test.ts | 35 +++++++----- apps/tests/src/utils/build-output-utils.ts | 53 +++++++++++++++++++ apps/tests/vitest.config.ts | 6 +++ .../start-nitro-v2-vite-plugin/src/index.ts | 1 + 9 files changed, 110 insertions(+), 54 deletions(-) create mode 100644 .changeset/shiny-clowns-clap.md create mode 100644 apps/tests/src/build/compressed-public-assets.server.test.ts create mode 100644 apps/tests/src/utils/build-output-utils.ts diff --git a/.changeset/shiny-clowns-clap.md b/.changeset/shiny-clowns-clap.md new file mode 100644 index 000000000..a55c0ee7b --- /dev/null +++ b/.changeset/shiny-clowns-clap.md @@ -0,0 +1,5 @@ +--- +"@solidjs/vite-plugin-nitro-2": patch +--- + +fix: compress public assets by default, matching `1.x` behavior diff --git a/apps/tests/src/build/compressed-public-assets.server.test.ts b/apps/tests/src/build/compressed-public-assets.server.test.ts new file mode 100644 index 000000000..2479f9902 --- /dev/null +++ b/apps/tests/src/build/compressed-public-assets.server.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { getBuildOutputDirs, getFiles } from "~/utils/build-output-utils"; + +describe("public assets compression", () => { + it("includes at least one .gz and one .br file in client output", async () => { + const { clientOutputRoot } = getBuildOutputDirs(); + const gzFiles = await getFiles(clientOutputRoot, /\.gz$/); + const brFiles = await getFiles(clientOutputRoot, /\.br$/); + + // Only files above 1KB are compressed, so we check that at least one .gz and one .br file exists + expect( + gzFiles.length, + `No .gz files found in client output: ${clientOutputRoot}`, + ).toBeGreaterThan(0); + expect( + brFiles.length, + `No .br files found in client output: ${clientOutputRoot}`, + ).toBeGreaterThan(0); + }); +}); diff --git a/apps/tests/src/routes/treeshaking/(no-side-effects).tsx b/apps/tests/src/routes/treeshaking/(no-side-effects).tsx index 343e7bb8f..40cff9649 100644 --- a/apps/tests/src/routes/treeshaking/(no-side-effects).tsx +++ b/apps/tests/src/routes/treeshaking/(no-side-effects).tsx @@ -1,6 +1,6 @@ import { createAsync } from "@solidjs/router"; -const a = 1; +const a = "myTreeshakingTestUniqueString1"; function getA() { return a; diff --git a/apps/tests/src/routes/treeshaking/server-secret-leak.server.test.ts b/apps/tests/src/routes/treeshaking/server-secret-leak.server.test.ts index 58fe66e50..7383bc694 100644 --- a/apps/tests/src/routes/treeshaking/server-secret-leak.server.test.ts +++ b/apps/tests/src/routes/treeshaking/server-secret-leak.server.test.ts @@ -1,8 +1,5 @@ -import { existsSync } from "node:fs"; -import { readdir, readFile } from "node:fs/promises"; -import path from "node:path"; -import { brotliDecompressSync, gunzipSync } from "node:zlib"; import { describe, expect, it } from "vitest"; +import { getBuildOutputDirs, getFiles, readFileContent } from "~/utils/build-output-utils"; // Avoid full pattern to exclude this file from scan const SECRET_MARKER = new RegExp(`${"MyServer"}${"SuperSecretUniqueString"}\\d+`, "g"); @@ -10,21 +7,7 @@ const ALL_FILE_EXTENSIONS = /\.(ts|tsx|js|jsx|mjs|cjs|mts|cts|css|map|gz|br)$/; describe("server code does not leak to client bundle", () => { it("verifies secret markers are server-only and not in client output", async () => { - const appRoot = process.cwd(); - const sourceRoot = path.join(appRoot, "src"); - const serverOutputRoot = path.join(appRoot, ".output/server"); - const clientOutputRoot = path.join(appRoot, ".output/public"); - - // Verify required directories exist - expect(existsSync(sourceRoot), `Source dir not found: ${sourceRoot}`).toBe(true); - expect( - existsSync(serverOutputRoot), - `Server output dir not found: ${serverOutputRoot}. Did you run the build? (pnpm --filter tests run build)`, - ).toBe(true); - expect( - existsSync(clientOutputRoot), - `Client output dir not found: ${clientOutputRoot}. Did you run the build? (pnpm --filter tests run build)`, - ).toBe(true); + const { sourceRoot, serverOutputRoot, clientOutputRoot } = getBuildOutputDirs(); // Collect and validate markers from source code const sourceMarkerCounts = await countSourceMarkers(sourceRoot); @@ -77,22 +60,3 @@ async function countSourceMarkers(rootDir: string) { } return markerCounts; } - -async function getFiles(dir: string, fileRegex: RegExp): Promise { - const entries = await readdir(dir, { recursive: true, withFileTypes: true }); - return entries - .filter(e => e.isFile() && fileRegex.test(e.name)) - .map(e => path.join(e.parentPath, e.name)); -} - -async function readFileContent(filePath: string) { - if (filePath.endsWith(".br")) { - return brotliDecompressSync(await readFile(filePath)).toString("utf-8"); - } - - if (filePath.endsWith(".gz")) { - return gunzipSync(await readFile(filePath)).toString("utf-8"); - } - - return readFile(filePath, "utf-8"); -} diff --git a/apps/tests/src/routes/treeshaking/side-effects.tsx b/apps/tests/src/routes/treeshaking/side-effects.tsx index 2a7a85153..9613d38ab 100644 --- a/apps/tests/src/routes/treeshaking/side-effects.tsx +++ b/apps/tests/src/routes/treeshaking/side-effects.tsx @@ -1,6 +1,6 @@ import { createAsync } from "@solidjs/router"; -export const a = 1; +export const a = "myTreeshakingTestUniqueString2"; function getA() { return a; diff --git a/apps/tests/src/routes/treeshaking/treeshake.server.test.ts b/apps/tests/src/routes/treeshaking/treeshake.server.test.ts index c1fe56750..9f963f449 100644 --- a/apps/tests/src/routes/treeshaking/treeshake.server.test.ts +++ b/apps/tests/src/routes/treeshaking/treeshake.server.test.ts @@ -1,23 +1,30 @@ -// I'm not sure if it's fair calling this a unit test, but let's go with that -import { readdir, readFile } from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; +import { getBuildOutputDirs, getFiles, readFileContent } from "~/utils/build-output-utils"; describe("Make sure treeshaking works", () => { it("should not have any unused code in the client-bundle", async () => { - const buildDir = path.resolve(process.cwd(), ".output/public/_build/assets"); - const files = await readdir(buildDir); - const targetFile = files.find( - file => file.startsWith("(no-side-effects)-") && file.endsWith(".js"), - ); - if (!targetFile) { - throw new Error("Treeshaking test: No target file not found"); + const { clientOutputRoot } = getBuildOutputDirs(); + const files = await getFiles(clientOutputRoot, /^\(no-side-effects\)-.*\.js(\.gz|\.br)?$/); + + expect(files.length, "No files matching the treeshaking pattern found").toBeGreaterThan(0); + + for (const targetFile of files) { + const file = await readFileContent(targetFile); + const result = file.includes("myTreeshakingTestUniqueString1"); + expect(result, `Unused code found in file: ${targetFile}`).toBeFalsy(); } - const file = await readFile(path.join(buildDir, targetFile), "utf-8"); + }); - const regex = /const a = 1;/g; - const result = regex.test(file); + it("should include side-effects code in the client-bundle", async () => { + const { clientOutputRoot } = getBuildOutputDirs(); + const files = await getFiles(clientOutputRoot, /^side-effects.*\.js(\.gz|\.br)?$/); - expect(result).toBeFalsy(); + expect(files.length, "No side-effects files matching the pattern found").toBeGreaterThan(0); + + for (const targetFile of files) { + const file = await readFileContent(targetFile); + const result = file.includes("myTreeshakingTestUniqueString2"); + expect(result, `Side-effects code not found in file: ${targetFile}`).toBeTruthy(); + } }); }); diff --git a/apps/tests/src/utils/build-output-utils.ts b/apps/tests/src/utils/build-output-utils.ts new file mode 100644 index 000000000..e9f215fed --- /dev/null +++ b/apps/tests/src/utils/build-output-utils.ts @@ -0,0 +1,53 @@ +import { existsSync } from "node:fs"; +import { readFile, readdir } from "node:fs/promises"; +import path from "node:path"; +import { brotliDecompressSync, gunzipSync } from "node:zlib"; + +export function getBuildOutputDirs() { + const appRoot = process.cwd(); + const sourceRoot = path.join(appRoot, "src"); + const serverOutputRoot = path.join(appRoot, ".output/server"); + const clientOutputRoot = path.join(appRoot, ".output/public"); + + if (!existsSync(sourceRoot)) { + throw new Error(`Source dir not found: ${sourceRoot}`); + } + + if (!existsSync(serverOutputRoot)) { + throw new Error( + `Server output dir not found: ${serverOutputRoot}. Did you run the build? (pnpm --filter tests run build)`, + ); + } + + if (!existsSync(clientOutputRoot)) { + throw new Error( + `Client output dir not found: ${clientOutputRoot}. Did you run the build? (pnpm --filter tests run build)`, + ); + } + + return { + sourceRoot, + serverOutputRoot, + clientOutputRoot, + }; +} + +export async function getFiles(dir: string, fileRegex: RegExp): Promise { + const entries = await readdir(dir, { recursive: true, withFileTypes: true }); + + return entries + .filter(e => e.isFile() && fileRegex.test(e.name)) + .map(e => path.join(e.parentPath, e.name)); +} + +export async function readFileContent(filePath: string) { + if (filePath.endsWith(".br")) { + return brotliDecompressSync(await readFile(filePath)).toString("utf-8"); + } + + if (filePath.endsWith(".gz")) { + return gunzipSync(await readFile(filePath)).toString("utf-8"); + } + + return readFile(filePath, "utf-8"); +} diff --git a/apps/tests/vitest.config.ts b/apps/tests/vitest.config.ts index 9255aca52..bdd7bf303 100644 --- a/apps/tests/vitest.config.ts +++ b/apps/tests/vitest.config.ts @@ -1,8 +1,14 @@ import solid from "vite-plugin-solid"; import { defineConfig } from "vitest/config"; import { playwright } from "@vitest/browser-playwright"; +import path from "path"; export default defineConfig({ + resolve: { + alias: { + "~": path.resolve(__dirname, "./src"), + }, + }, plugins: [solid()], test: { mockReset: true, diff --git a/packages/start-nitro-v2-vite-plugin/src/index.ts b/packages/start-nitro-v2-vite-plugin/src/index.ts index cc2ec4a55..7db44d76b 100644 --- a/packages/start-nitro-v2-vite-plugin/src/index.ts +++ b/packages/start-nitro-v2-vite-plugin/src/index.ts @@ -82,6 +82,7 @@ export function nitroV2Plugin(nitroConfig?: UserNitroConfig): PluginOption { generateTsConfig: false, generateRuntimeConfigTypes: false, }, + compressPublicAssets: true, ...nitroConfig, dev: false, routeRules: {