From 7a625724e240c05f3b1b1b000bc5f51333982522 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Sat, 4 May 2024 20:42:53 +0800 Subject: [PATCH] fix: configure Vite config `optimizeDeps.entries` option (#12182) * add failing test * add optimizeDeps.entries * changeset * prettier * only match + prefixed route files * try to fix test for ubuntu firefox * hope this works * nicer comment...? * move test to dev-only test app * prettier * oops forgot to undo this * add tests for each routing file * prettier --- .changeset/stale-otters-yell.md | 5 + packages/kit/src/exports/vite/index.js | 1 + .../_test_dependencies/cjs-only/index.js | 3 + .../_test_dependencies/cjs-only/package.json | 10 ++ packages/kit/test/apps/dev-only/package.json | 10 ++ .../test/apps/dev-only/src/hooks.client.js | 7 ++ packages/kit/test/apps/dev-only/src/hooks.js | 7 ++ .../apps/dev-only/src/routes/+page.svelte | 13 +++ .../src/routes/optimize-deps/+error.svelte | 8 ++ .../src/routes/optimize-deps/+layout.js | 4 + .../routes/optimize-deps/+layout.server.js | 4 + .../src/routes/optimize-deps/+layout.svelte | 8 ++ .../src/routes/optimize-deps/+page.js | 4 + .../src/routes/optimize-deps/+page.server.js | 4 + .../src/routes/optimize-deps/+page.svelte | 6 ++ .../src/routes/optimize-deps/+server.js | 6 ++ packages/kit/test/apps/dev-only/test/test.js | 99 +++++++++++++++++++ .../kit/test/apps/dev-only/vite.config.js | 2 +- packages/kit/test/utils.js | 2 +- pnpm-lock.yaml | 35 +++++++ 20 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 .changeset/stale-otters-yell.md create mode 100644 packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/index.js create mode 100644 packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/package.json create mode 100644 packages/kit/test/apps/dev-only/src/hooks.client.js create mode 100644 packages/kit/test/apps/dev-only/src/hooks.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/+page.svelte create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+error.svelte create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.server.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.svelte create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.server.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.svelte create mode 100644 packages/kit/test/apps/dev-only/src/routes/optimize-deps/+server.js diff --git a/.changeset/stale-otters-yell.md b/.changeset/stale-otters-yell.md new file mode 100644 index 000000000000..0a9161e9685f --- /dev/null +++ b/.changeset/stale-otters-yell.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +fix: prevent excessive Vite dependency optimizations on navigation diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js index e80fb786aca9..c91055ba9aa7 100644 --- a/packages/kit/src/exports/vite/index.js +++ b/packages/kit/src/exports/vite/index.js @@ -293,6 +293,7 @@ async function kit({ svelte_config }) { cors: { preflightContinue: true } }, optimizeDeps: { + entries: [`${kit.files.routes}/**/+*.{svelte,js,ts}`], exclude: [ '@sveltejs/kit', // exclude kit features so that libraries using them work even when they are prebundled diff --git a/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/index.js b/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/index.js new file mode 100644 index 000000000000..41cb0f071f46 --- /dev/null +++ b/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/index.js @@ -0,0 +1,3 @@ +module.exports = { + cjs: () => 'cjs' +}; diff --git a/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/package.json b/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/package.json new file mode 100644 index 000000000000..c4342246d620 --- /dev/null +++ b/packages/kit/test/apps/dev-only/_test_dependencies/cjs-only/package.json @@ -0,0 +1,10 @@ +{ + "version": "1.0.0", + "private": true, + "name": "e2e-test-dep-cjs-only", + "main": "index.js", + "files": [ + "index.js" + ], + "type": "commonjs" +} diff --git a/packages/kit/test/apps/dev-only/package.json b/packages/kit/test/apps/dev-only/package.json index 148e92fde2bf..f26c7ae0c15f 100644 --- a/packages/kit/test/apps/dev-only/package.json +++ b/packages/kit/test/apps/dev-only/package.json @@ -13,6 +13,16 @@ "@sveltejs/kit": "workspace:^", "@sveltejs/vite-plugin-svelte": "^3.0.1", "cross-env": "^7.0.3", + "e2e-test-dep-page-svelte": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-page-universal": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-page-server": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-layout-svelte": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-layout-universal": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-layout-server": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-server": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-error": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-hooks": "file:./_test_dependencies/cjs-only", + "e2e-test-dep-hooks-client": "file:./_test_dependencies/cjs-only", "svelte": "^4.2.10", "svelte-check": "^3.6.3", "typescript": "^5.3.3", diff --git a/packages/kit/test/apps/dev-only/src/hooks.client.js b/packages/kit/test/apps/dev-only/src/hooks.client.js new file mode 100644 index 000000000000..2f9818a0499d --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/hooks.client.js @@ -0,0 +1,7 @@ +import cjs from 'e2e-test-dep-hooks-client'; +cjs.cjs(); + +/** @type {import("@sveltejs/kit").HandleClientError} */ +export function handleError({ message }) { + return { message }; +} diff --git a/packages/kit/test/apps/dev-only/src/hooks.js b/packages/kit/test/apps/dev-only/src/hooks.js new file mode 100644 index 000000000000..b8ad0e2c48d0 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/hooks.js @@ -0,0 +1,7 @@ +import cjs from 'e2e-test-dep-hooks'; +cjs.cjs(); + +/** @type {import("@sveltejs/kit").Reroute} */ +export function reroute({ url }) { + return url.pathname; +} diff --git a/packages/kit/test/apps/dev-only/src/routes/+page.svelte b/packages/kit/test/apps/dev-only/src/routes/+page.svelte new file mode 100644 index 000000000000..0ec33b5fa29c --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/+page.svelte @@ -0,0 +1,13 @@ + + +

{message}

+ +Go to /optimize-deps diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+error.svelte b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+error.svelte new file mode 100644 index 000000000000..c8e1016c5187 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+error.svelte @@ -0,0 +1,8 @@ + + + + +

this error page uses a new dependency

diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.js b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.js new file mode 100644 index 000000000000..58dffdc26766 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.js @@ -0,0 +1,4 @@ +import cjs from 'e2e-test-dep-layout-universal'; +cjs.cjs(); + +export function load() {} diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.server.js b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.server.js new file mode 100644 index 000000000000..fdbf6d7f68e7 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.server.js @@ -0,0 +1,4 @@ +import cjs from 'e2e-test-dep-layout-server'; +cjs.cjs(); + +export function load() {} diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.svelte b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.svelte new file mode 100644 index 000000000000..28c1358eb075 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+layout.svelte @@ -0,0 +1,8 @@ + + + + +

this layout uses a new dependency

diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.js b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.js new file mode 100644 index 000000000000..70077f6aa012 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.js @@ -0,0 +1,4 @@ +import cjs from 'e2e-test-dep-page-universal'; +cjs.cjs(); + +export function load() {} diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.server.js b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.server.js new file mode 100644 index 000000000000..448d7b8efe5d --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.server.js @@ -0,0 +1,4 @@ +import cjs from 'e2e-test-dep-page-server'; +cjs.cjs(); + +export function load() {} diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.svelte b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.svelte new file mode 100644 index 000000000000..ff1cb449ee24 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+page.svelte @@ -0,0 +1,6 @@ + + +

this page uses a new dependency

diff --git a/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+server.js b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+server.js new file mode 100644 index 000000000000..3168455aadf6 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/optimize-deps/+server.js @@ -0,0 +1,6 @@ +import cjs from 'e2e-test-dep-server'; +cjs.cjs(); + +export function GET() { + return new Response('ok'); +} diff --git a/packages/kit/test/apps/dev-only/test/test.js b/packages/kit/test/apps/dev-only/test/test.js index 124d4188a2f7..346043f09d80 100644 --- a/packages/kit/test/apps/dev-only/test/test.js +++ b/packages/kit/test/apps/dev-only/test/test.js @@ -1,5 +1,10 @@ import { expect } from '@playwright/test'; import { test } from '../../../utils.js'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); /** @typedef {import('@playwright/test').Response} Response */ @@ -78,3 +83,97 @@ test.describe.serial('Illegal imports', () => { ); }); }); + +test.describe('Vite', () => { + test.skip(({ javaScriptEnabled }) => !process.env.DEV || !javaScriptEnabled); + + test('optimizes +page.svelte dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-page-svelte'); + }); + + test('optimizes +page.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-page-universal'); + }); + + test('optimizes +page.server.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-page-server'); + }); + + test('optimizes +layout.svelte dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-layout-svelte'); + }); + + test('optimizes +layout.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-layout-universal'); + }); + + test('optimizes +layout.server.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-layout-server'); + }); + + test('optimizes +error.svelte dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-error'); + }); + + test('optimizes hooks.client.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-hooks-client'); + }); + + test('optimizes hooks.js dependencies', async ({ page }) => { + await page.goto('/'); + await page.getByText('hello world!').waitFor(); + + const manifest_path = path.join(__dirname, '../node_modules/.vite/deps/_metadata.json'); + const manifest = JSON.parse(fs.readFileSync(manifest_path, 'utf-8')); + + expect(manifest).toHaveProperty('optimized.e2e-test-dep-hooks'); + }); +}); diff --git a/packages/kit/test/apps/dev-only/vite.config.js b/packages/kit/test/apps/dev-only/vite.config.js index 0afea7f14a85..d6742f261346 100644 --- a/packages/kit/test/apps/dev-only/vite.config.js +++ b/packages/kit/test/apps/dev-only/vite.config.js @@ -10,7 +10,7 @@ const config = { optimizeDeps: { // for CI, we need to explicitly prebundle deps, since // the reload confuses Playwright - include: ['cookie', 'marked'] + include: ['cookie'] }, plugins: [sveltekit()], server: { diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 05b88c9ce0c6..38e4faaedd6c 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -242,7 +242,7 @@ export const config = { // generous timeouts on CI timeout: process.env.CI ? 45000 : 15000, webServer: { - command: process.env.DEV ? 'pnpm dev' : 'pnpm build && pnpm preview', + command: process.env.DEV ? 'pnpm dev --force' : 'pnpm build && pnpm preview', port: process.env.DEV ? 5173 : 4173 }, retries: process.env.CI ? 2 : 0, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e5593308d142..eabe9ab029a8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -555,6 +555,36 @@ importers: cross-env: specifier: ^7.0.3 version: 7.0.3 + e2e-test-dep-error: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-hooks: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-hooks-client: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-layout-server: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-layout-svelte: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-layout-universal: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-page-server: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-page-svelte: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-page-universal: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only + e2e-test-dep-server: + specifier: file:./_test_dependencies/cjs-only + version: file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only svelte: specifier: ^4.2.10 version: 4.2.14 @@ -7473,3 +7503,8 @@ packages: /zod@3.22.4: resolution: {integrity: sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==} dev: false + + file:packages/kit/test/apps/dev-only/_test_dependencies/cjs-only: + resolution: {directory: packages/kit/test/apps/dev-only/_test_dependencies/cjs-only, type: directory} + name: e2e-test-dep-cjs-only + dev: true