From f2aed62d0bf1b66e870ee6b4aab80cd1702793ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Padier?= Date: Wed, 15 Jan 2025 09:33:12 +0100 Subject: [PATCH 01/38] fix: tree shake stringified JSON imports (#19189) --- .../vite/src/node/__tests__/build.spec.ts | 63 +++++++++++++++++++ .../src/node/__tests__/plugins/json.spec.ts | 6 +- packages/vite/src/node/plugins/json.ts | 4 +- 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/packages/vite/src/node/__tests__/build.spec.ts b/packages/vite/src/node/__tests__/build.spec.ts index 3c4be301e6d528..f5dd1419f04d7c 100644 --- a/packages/vite/src/node/__tests__/build.spec.ts +++ b/packages/vite/src/node/__tests__/build.spec.ts @@ -134,6 +134,69 @@ describe('build', () => { assertOutputHashContentChange(result[0], result[1]) }) + test.for([ + [true, true], + [true, false], + [false, true], + [false, false], + ['auto', true], + ['auto', false], + ] as const)( + 'large json object files should have tree-shaking (json.stringify: %s, json.namedExports: %s)', + async ([stringify, namedExports]) => { + const esBundle = (await build({ + mode: 'development', + root: resolve(__dirname, 'packages/build-project'), + logLevel: 'silent', + json: { stringify, namedExports }, + build: { + minify: false, + modulePreload: { polyfill: false }, + write: false, + }, + plugins: [ + { + name: 'test', + resolveId(id) { + if ( + id === 'entry.js' || + id === 'object.json' || + id === 'array.json' + ) { + return '\0' + id + } + }, + load(id) { + if (id === '\0entry.js') { + return ` + import object from 'object.json'; + import array from 'array.json'; + console.log(); + ` + } + if (id === '\0object.json') { + return ` + {"value": {"${stringify}_${namedExports}":"JSON_OBJ${'_'.repeat(10_000)}"}} + ` + } + if (id === '\0array.json') { + return ` + ["${stringify}_${namedExports}","JSON_ARR${'_'.repeat(10_000)}"] + ` + } + }, + }, + ], + })) as RollupOutput + + const foo = esBundle.output.find( + (chunk) => chunk.type === 'chunk' && chunk.isEntry, + ) as OutputChunk + expect(foo.code).not.contains('JSON_ARR') + expect(foo.code).not.contains('JSON_OBJ') + }, + ) + test('external modules should not be hoisted in library build', async () => { const [esBundle] = (await build({ logLevel: 'silent', diff --git a/packages/vite/src/node/__tests__/plugins/json.spec.ts b/packages/vite/src/node/__tests__/plugins/json.spec.ts index 3e114fd3dd145b..e90bcb39c22737 100644 --- a/packages/vite/src/node/__tests__/plugins/json.spec.ts +++ b/packages/vite/src/node/__tests__/plugins/json.spec.ts @@ -62,7 +62,7 @@ describe('transform', () => { false, ) expect(actualSmall).toMatchInlineSnapshot( - `"export default JSON.parse("[{\\"a\\":1,\\"b\\":2}]")"`, + `"export default /* #__PURE__ */ JSON.parse("[{\\"a\\":1,\\"b\\":2}]")"`, ) }) @@ -122,7 +122,7 @@ describe('transform', () => { false, ) expect(actualDev).toMatchInlineSnapshot( - `"export default JSON.parse("{\\"a\\":1,\\n\\"🫠\\": \\"\\",\\n\\"const\\": false}")"`, + `"export default /* #__PURE__ */ JSON.parse("{\\"a\\":1,\\n\\"🫠\\": \\"\\",\\n\\"const\\": false}")"`, ) const actualBuild = transform( @@ -131,7 +131,7 @@ describe('transform', () => { true, ) expect(actualBuild).toMatchInlineSnapshot( - `"export default JSON.parse("{\\"a\\":1,\\"🫠\\":\\"\\",\\"const\\":false}")"`, + `"export default /* #__PURE__ */ JSON.parse("{\\"a\\":1,\\"🫠\\":\\"\\",\\"const\\":false}")"`, ) }) diff --git a/packages/vite/src/node/plugins/json.ts b/packages/vite/src/node/plugins/json.ts index 33c1c0d27aeaac..0234d471d39243 100644 --- a/packages/vite/src/node/plugins/json.ts +++ b/packages/vite/src/node/plugins/json.ts @@ -87,7 +87,7 @@ export function jsonPlugin( } return { - code: `export default JSON.parse(${JSON.stringify(json)})`, + code: `export default /* #__PURE__ */ JSON.parse(${JSON.stringify(json)})`, map: { mappings: '' }, } } @@ -120,7 +120,7 @@ function serializeValue(value: unknown): string { value != null && valueAsString.length > 10 * 1000 ) { - return `JSON.parse(${JSON.stringify(valueAsString)})` + return `/* #__PURE__ */ JSON.parse(${JSON.stringify(valueAsString)})` } return valueAsString } From 4f5845a3182fc950eb9cd76d7161698383113b18 Mon Sep 17 00:00:00 2001 From: pacexy Date: Thu, 16 Jan 2025 16:41:58 +0800 Subject: [PATCH 02/38] docs: unify `__dirname` usage (#19176) --- docs/guide/api-environment-frameworks.md | 5 +++++ docs/guide/build.md | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/docs/guide/api-environment-frameworks.md b/docs/guide/api-environment-frameworks.md index 6c16d934486ddd..00493c0c844129 100644 --- a/docs/guide/api-environment-frameworks.md +++ b/docs/guide/api-environment-frameworks.md @@ -46,8 +46,13 @@ The `runner` is evaluated eagerly when it's accessed for the first time. Beware Given a Vite server configured in middleware mode as described by the [SSR setup guide](/guide/ssr#setting-up-the-dev-server), let's implement the SSR middleware using the environment API. Error handling is omitted. ```js +import fs from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' import { createServer } from 'vite' +const __dirname = path.dirname(fileURLToPath(import.meta.url)) + const server = await createServer({ server: { middlewareMode: true }, appType: 'custom', diff --git a/docs/guide/build.md b/docs/guide/build.md index e08ad32b247f72..a91ce95906c721 100644 --- a/docs/guide/build.md +++ b/docs/guide/build.md @@ -106,9 +106,12 @@ During dev, simply navigate or link to `/nested/` - it works as expected, just l During build, all you need to do is to specify multiple `.html` files as entry points: ```js twoslash [vite.config.js] -import { resolve } from 'path' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' import { defineConfig } from 'vite' +const __dirname = dirname(fileURLToPath(import.meta.url)) + export default defineConfig({ build: { rollupOptions: { @@ -134,9 +137,12 @@ When it is time to bundle your library for distribution, use the [`build.lib` co ::: code-group ```js twoslash [vite.config.js (single entry)] -import { resolve } from 'path' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' import { defineConfig } from 'vite' +const __dirname = dirname(fileURLToPath(import.meta.url)) + export default defineConfig({ build: { lib: { @@ -162,9 +168,12 @@ export default defineConfig({ ``` ```js twoslash [vite.config.js (multiple entries)] -import { resolve } from 'path' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' import { defineConfig } from 'vite' +const __dirname = dirname(fileURLToPath(import.meta.url)) + export default defineConfig({ build: { lib: { From c0f72a695c5308cba605e3db4f851f4f6692e50c Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Mon, 20 Jan 2025 16:55:34 +0900 Subject: [PATCH 03/38] release: v6.0.8 --- packages/vite/CHANGELOG.md | 16 ++++++++++++++++ packages/vite/package.json | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/vite/CHANGELOG.md b/packages/vite/CHANGELOG.md index 4ac3e5c274f8f8..df2e8d326fea35 100644 --- a/packages/vite/CHANGELOG.md +++ b/packages/vite/CHANGELOG.md @@ -1,3 +1,19 @@ +## 6.0.8 (2025-01-20) + +* fix: avoid SSR HMR for HTML files (#19193) ([3bd55bc](https://github.com/vitejs/vite/commit/3bd55bcb7e831d2c4f66c90d7bbb3e1fbf7a02b6)), closes [#19193](https://github.com/vitejs/vite/issues/19193) +* fix: build time display 7m 60s (#19108) ([cf0d2c8](https://github.com/vitejs/vite/commit/cf0d2c8e232a1af716c71cdd2218d180f7ecc02b)), closes [#19108](https://github.com/vitejs/vite/issues/19108) +* fix: don't resolve URL starting with double slash (#19059) ([35942cd](https://github.com/vitejs/vite/commit/35942cde11fd8a68fa89bf25f7aa1ddb87d775b2)), closes [#19059](https://github.com/vitejs/vite/issues/19059) +* fix: ensure `server.close()` only called once (#19204) ([db81c2d](https://github.com/vitejs/vite/commit/db81c2dada961f40c0882b5182adf2f34bb5c178)), closes [#19204](https://github.com/vitejs/vite/issues/19204) +* fix: resolve.conditions in ResolvedConfig was `defaultServerConditions` (#19174) ([ad75c56](https://github.com/vitejs/vite/commit/ad75c56dce5618a3a416e18f9a5c3880d437a107)), closes [#19174](https://github.com/vitejs/vite/issues/19174) +* fix: tree shake stringified JSON imports (#19189) ([f2aed62](https://github.com/vitejs/vite/commit/f2aed62d0bf1b66e870ee6b4aab80cd1702793ab)), closes [#19189](https://github.com/vitejs/vite/issues/19189) +* fix: use shared sigterm callback (#19203) ([47039f4](https://github.com/vitejs/vite/commit/47039f4643179be31a8d7c7fbff83c5c13deb787)), closes [#19203](https://github.com/vitejs/vite/issues/19203) +* fix(deps): update all non-major dependencies (#19098) ([8639538](https://github.com/vitejs/vite/commit/8639538e6498d1109da583ad942c1472098b5919)), closes [#19098](https://github.com/vitejs/vite/issues/19098) +* fix(optimizer): use correct default install state path for yarn PnP (#19119) ([e690d8b](https://github.com/vitejs/vite/commit/e690d8bb1e5741e81df5b7a6a5c8c3c1c971fa41)), closes [#19119](https://github.com/vitejs/vite/issues/19119) +* fix(types): improve `ESBuildOptions.include / exclude` type to allow `readonly (string | RegExp)[]` ([ea53e70](https://github.com/vitejs/vite/commit/ea53e7095297ea4192490fd58556414cc59a8975)), closes [#19146](https://github.com/vitejs/vite/issues/19146) +* chore(deps): update dependency pathe to v2 (#19139) ([71506f0](https://github.com/vitejs/vite/commit/71506f0a8deda5254cb49c743cd439dfe42859ce)), closes [#19139](https://github.com/vitejs/vite/issues/19139) + + + ## 6.0.7 (2025-01-02) * fix: fix `minify` when `builder.sharedPlugins: true` (#19025) ([f7b1964](https://github.com/vitejs/vite/commit/f7b1964d3a93a21f80b61638fa6ae9606d0a6f4f)), closes [#19025](https://github.com/vitejs/vite/issues/19025) diff --git a/packages/vite/package.json b/packages/vite/package.json index 0725bcdea9e536..2dbe1460da35dc 100644 --- a/packages/vite/package.json +++ b/packages/vite/package.json @@ -1,6 +1,6 @@ { "name": "vite", - "version": "6.0.7", + "version": "6.0.8", "type": "module", "license": "MIT", "author": "Evan You", From b09572acc939351f4e4c50ddf793017a92c678b1 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:04:31 +0900 Subject: [PATCH 04/38] fix!: default `server.cors: false` to disallow fetching from untrusted origins --- docs/config/server-options.md | 9 ++- docs/guide/backend-integration.md | 6 ++ .../vite/src/node/__tests__/config.spec.ts | 4 +- packages/vite/src/node/http.ts | 12 +++ packages/vite/src/node/server/index.ts | 4 +- .../fs-serve/__tests__/fs-serve.spec.ts | 77 ++++++++++++++++++- playground/fs-serve/root/src/code.js | 1 + playground/fs-serve/root/src/index.html | 1 + 8 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 playground/fs-serve/root/src/code.js diff --git a/docs/config/server-options.md b/docs/config/server-options.md index 9aa26b1aca79af..92a48f8570273f 100644 --- a/docs/config/server-options.md +++ b/docs/config/server-options.md @@ -147,8 +147,15 @@ export default defineConfig({ ## server.cors - **Type:** `boolean | CorsOptions` +- **Default:** `false` + +Configure CORS for the dev server. Pass an [options object](https://github.com/expressjs/cors#configuration-options) to fine tune the behavior or `true` to allow any origin. + +:::warning -Configure CORS for the dev server. This is enabled by default and allows any origin. Pass an [options object](https://github.com/expressjs/cors#configuration-options) to fine tune the behavior or `false` to disable. +We recommend setting a specific value rather than `true` to avoid exposing the source code to untrusted origins. + +::: ## server.headers diff --git a/docs/guide/backend-integration.md b/docs/guide/backend-integration.md index 5f3cc938774530..34319601f94b67 100644 --- a/docs/guide/backend-integration.md +++ b/docs/guide/backend-integration.md @@ -12,6 +12,12 @@ If you need a custom integration, you can follow the steps in this guide to conf import { defineConfig } from 'vite' // ---cut--- export default defineConfig({ + server: { + cors: { + // the origin you will be accessing via browser + origin: 'http://my-backend.example.com', + }, + }, build: { // generate .vite/manifest.json in outDir manifest: true, diff --git a/packages/vite/src/node/__tests__/config.spec.ts b/packages/vite/src/node/__tests__/config.spec.ts index f249a06fa23e56..e2966f262add71 100644 --- a/packages/vite/src/node/__tests__/config.spec.ts +++ b/packages/vite/src/node/__tests__/config.spec.ts @@ -249,7 +249,7 @@ describe('preview config', () => { 'Cache-Control': 'no-store', }, proxy: { '/foo': 'http://localhost:4567' }, - cors: false, + cors: true, }) test('preview inherits server config with default port', async () => { @@ -285,7 +285,7 @@ describe('preview config', () => { open: false, host: false, proxy: { '/bar': 'http://localhost:3010' }, - cors: true, + cors: false, }) test('preview overrides server config', async () => { diff --git a/packages/vite/src/node/http.ts b/packages/vite/src/node/http.ts index ec1bf5e645641d..faaf6e17d17078 100644 --- a/packages/vite/src/node/http.ts +++ b/packages/vite/src/node/http.ts @@ -59,8 +59,14 @@ export interface CommonServerOptions { /** * Configure CORS for the dev server. * Uses https://github.com/expressjs/cors. + * + * When enabling this option, **we recommend setting a specific value + * rather than `true`** to avoid exposing the source code to untrusted origins. + * * Set to `true` to allow all methods from any origin, or configure separately * using an object. + * + * @default false */ cors?: CorsOptions | boolean /** @@ -73,6 +79,12 @@ export interface CommonServerOptions { * https://github.com/expressjs/cors#configuration-options */ export interface CorsOptions { + /** + * Configures the Access-Control-Allow-Origin CORS header. + * + * **We recommend setting a specific value rather than + * `true`** to avoid exposing the source code to untrusted origins. + */ origin?: | CorsOrigin | (( diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index 486b245d9c71de..88d5fed1df98df 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -851,7 +851,7 @@ export async function _createServer( middlewares.use(timeMiddleware(root)) } - // cors (enabled by default) + // cors const { cors } = serverConfig if (cors !== false) { middlewares.use(corsMiddleware(typeof cors === 'boolean' ? {} : cors)) @@ -1046,7 +1046,7 @@ export const serverConfigDefaults = Object.freeze({ https: undefined, open: false, proxy: undefined, - cors: true, + cors: false, headers: {}, // hmr // ws diff --git a/playground/fs-serve/__tests__/fs-serve.spec.ts b/playground/fs-serve/__tests__/fs-serve.spec.ts index 16ecc0b78dc295..b5f799b358b94a 100644 --- a/playground/fs-serve/__tests__/fs-serve.spec.ts +++ b/playground/fs-serve/__tests__/fs-serve.spec.ts @@ -1,14 +1,27 @@ import fetch from 'node-fetch' -import { beforeAll, describe, expect, test } from 'vitest' +import { + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, +} from 'vitest' +import type { Page } from 'playwright-chromium' import testJSON from '../safe.json' -import { isServe, page, viteTestUrl } from '~utils' +import { browser, isServe, page, viteTestUrl } from '~utils' + +const getViteTestIndexHtmlUrl = () => { + const srcPrefix = viteTestUrl.endsWith('/') ? '' : '/' + // NOTE: viteTestUrl is set lazily + return viteTestUrl + srcPrefix + 'src/' +} const stringified = JSON.stringify(testJSON) describe.runIf(isServe)('main', () => { beforeAll(async () => { - const srcPrefix = viteTestUrl.endsWith('/') ? '' : '/' - await page.goto(viteTestUrl + srcPrefix + 'src/') + await page.goto(getViteTestIndexHtmlUrl()) }) test('default import', async () => { @@ -113,3 +126,59 @@ describe('fetch', () => { expect(res.headers.get('x-served-by')).toBe('vite') }) }) + +describe('cross origin', () => { + const fetchStatusFromPage = async (page: Page, url: string) => { + return await page.evaluate(async (url: string) => { + try { + const res = await globalThis.fetch(url) + return res.status + } catch { + return -1 + } + }, url) + } + + describe('allowed for same origin', () => { + beforeEach(async () => { + await page.goto(getViteTestIndexHtmlUrl()) + }) + + test('fetch HTML file', async () => { + const status = await fetchStatusFromPage(page, viteTestUrl + '/src/') + expect(status).toBe(200) + }) + + test.runIf(isServe)('fetch JS file', async () => { + const status = await fetchStatusFromPage( + page, + viteTestUrl + '/src/code.js', + ) + expect(status).toBe(200) + }) + }) + + describe('denied for different origin', async () => { + let page2: Page + beforeEach(async () => { + page2 = await browser.newPage() + await page2.goto('http://vite.dev/404') + }) + afterEach(async () => { + await page2.close() + }) + + test('fetch HTML file', async () => { + const status = await fetchStatusFromPage(page2, viteTestUrl + '/src/') + expect(status).not.toBe(200) + }) + + test.runIf(isServe)('fetch JS file', async () => { + const status = await fetchStatusFromPage( + page2, + viteTestUrl + '/src/code.js', + ) + expect(status).not.toBe(200) + }) + }) +}) diff --git a/playground/fs-serve/root/src/code.js b/playground/fs-serve/root/src/code.js new file mode 100644 index 00000000000000..33fd8df878207b --- /dev/null +++ b/playground/fs-serve/root/src/code.js @@ -0,0 +1 @@ +// code.js diff --git a/playground/fs-serve/root/src/index.html b/playground/fs-serve/root/src/index.html index fb1276d79fea22..9f56c8831c12d9 100644 --- a/playground/fs-serve/root/src/index.html +++ b/playground/fs-serve/root/src/index.html @@ -52,6 +52,7 @@

Denied

@@ -24,6 +25,7 @@
+
diff --git a/playground/hmr/modules.d.ts b/playground/hmr/modules.d.ts index 122559a692ef20..e880082076b638 100644 --- a/playground/hmr/modules.d.ts +++ b/playground/hmr/modules.d.ts @@ -1,3 +1,7 @@ declare module 'virtual:file' { export const virtual: string } + +declare module 'virtual:file-dep' { + export const virtual: string +} diff --git a/playground/hmr/vite.config.ts b/playground/hmr/vite.config.ts index 94b07092b58f15..9ee8024ee2bf44 100644 --- a/playground/hmr/vite.config.ts +++ b/playground/hmr/vite.config.ts @@ -45,23 +45,22 @@ function virtualPlugin(): Plugin { return { name: 'virtual-file', resolveId(id) { - if (id === 'virtual:file') { - return '\0virtual:file' + if (id.startsWith('virtual:file')) { + return '\0' + id } }, load(id) { - if (id === '\0virtual:file') { + if (id.startsWith('\0virtual:file')) { return `\ import { virtual as _virtual } from "/importedVirtual.js"; export const virtual = _virtual + '${num}';` } }, configureServer(server) { - server.environments.client.hot.on('virtual:increment', async () => { - const mod = - await server.environments.client.moduleGraph.getModuleByUrl( - '\0virtual:file', - ) + server.environments.client.hot.on('virtual:increment', async (suffix) => { + const mod = await server.environments.client.moduleGraph.getModuleById( + '\0virtual:file' + (suffix || ''), + ) if (mod) { num++ server.environments.client.reloadModule(mod) From 2c2d521abfd7a3263b5082f9420738ad0ef67c71 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Thu, 23 Jan 2025 14:19:34 +0000 Subject: [PATCH 35/38] feat: add the `builtins` environment `resolve` (#18584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ç¿  / green --- .../vite/src/node/__tests__/resolve.spec.ts | 133 +++++++++++++++++- packages/vite/src/node/config.ts | 12 +- packages/vite/src/node/external.ts | 4 +- .../src/node/optimizer/esbuildDepPlugin.ts | 2 +- .../vite/src/node/plugins/importAnalysis.ts | 2 +- packages/vite/src/node/plugins/resolve.ts | 112 +++++++++------ packages/vite/src/node/ssr/fetchModule.ts | 7 +- packages/vite/src/node/utils.ts | 42 +++++- 8 files changed, 254 insertions(+), 60 deletions(-) diff --git a/packages/vite/src/node/__tests__/resolve.spec.ts b/packages/vite/src/node/__tests__/resolve.spec.ts index 111cdb99a6b1a0..ad15dcdd73a547 100644 --- a/packages/vite/src/node/__tests__/resolve.spec.ts +++ b/packages/vite/src/node/__tests__/resolve.spec.ts @@ -2,7 +2,7 @@ import { join } from 'node:path' import { describe, expect, onTestFinished, test } from 'vitest' import { createServer } from '../server' import { createServerModuleRunner } from '../ssr/runtime/serverModuleRunner' -import type { InlineConfig } from '../config' +import type { EnvironmentOptions, InlineConfig } from '../config' import { build } from '../build' describe('import and resolveId', () => { @@ -116,6 +116,137 @@ describe('file url', () => { expect(mod4.default).toBe(mod) }) + describe('environment builtins', () => { + function getConfig( + targetEnv: 'client' | 'ssr' | string, + builtins: NonNullable['builtins'], + ): InlineConfig { + return { + configFile: false, + root: join(import.meta.dirname, 'fixtures/file-url'), + logLevel: 'error', + server: { + middlewareMode: true, + }, + environments: { + [targetEnv]: { + resolve: { + builtins, + }, + }, + }, + } + } + + async function run({ + builtins, + targetEnv = 'custom', + testEnv = 'custom', + idToResolve, + }: { + builtins?: NonNullable['builtins'] + targetEnv?: 'client' | 'ssr' | string + testEnv?: 'client' | 'ssr' | string + idToResolve: string + }) { + const server = await createServer(getConfig(targetEnv, builtins)) + onTestFinished(() => server.close()) + + return server.environments[testEnv]?.pluginContainer.resolveId( + idToResolve, + ) + } + + test('declared builtin string', async () => { + const resolved = await run({ + builtins: ['my-env:custom-builtin'], + idToResolve: 'my-env:custom-builtin', + }) + expect(resolved?.external).toBe(true) + }) + + test('declared builtin regexp', async () => { + const resolved = await run({ + builtins: [/^my-env:\w/], + idToResolve: 'my-env:custom-builtin', + }) + expect(resolved?.external).toBe(true) + }) + + test('non declared builtin', async () => { + const resolved = await run({ + builtins: [ + /* empty */ + ], + idToResolve: 'my-env:custom-builtin', + }) + expect(resolved).toBeNull() + }) + + test('non declared node builtin', async () => { + await expect( + run({ + builtins: [ + /* empty */ + ], + idToResolve: 'node:fs', + }), + ).rejects.toThrowError( + /Automatically externalized node built-in module "node:fs"/, + ) + }) + + test('default to node-like builtins', async () => { + const resolved = await run({ + idToResolve: 'node:fs', + }) + expect(resolved?.external).toBe(true) + }) + + test('default to node-like builtins for ssr environment', async () => { + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'ssr', + }) + expect(resolved?.external).toBe(true) + }) + + test('no default to node-like builtins for client environment', async () => { + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'client', + }) + expect(resolved?.id).toEqual('__vite-browser-external:node:fs') + }) + + test('no builtins overriding for client environment', async () => { + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'client', + targetEnv: 'client', + }) + expect(resolved?.id).toEqual('__vite-browser-external:node:fs') + }) + + test('declared node builtin', async () => { + const resolved = await run({ + builtins: [/^node:/], + idToResolve: 'node:fs', + }) + expect(resolved?.external).toBe(true) + }) + + test('declared builtin string in different environment', async () => { + const resolved = await run({ + builtins: ['my-env:custom-builtin'], + idToResolve: 'my-env:custom-builtin', + targetEnv: 'custom', + testEnv: 'ssr', + }) + expect(resolved).toBe(null) + }) + }) + test('build', async () => { await build({ ...getConfig(), diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 493e1b7ec60dc0..1669e2b2920768 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -63,16 +63,17 @@ import { asyncFlatten, createDebugger, createFilter, - isBuiltin, isExternalUrl, isFilePathESM, isInNodeModules, isNodeBuiltin, + isNodeLikeBuiltin, isObject, isParentDirectory, mergeAlias, mergeConfig, mergeWithDefaults, + nodeLikeBuiltins, normalizeAlias, normalizePath, } from './utils' @@ -919,7 +920,11 @@ function resolveEnvironmentResolveOptions( isSsrTargetWebworkerEnvironment ? DEFAULT_CLIENT_CONDITIONS : DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'), - enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment, + builtins: + resolve?.builtins ?? + (consumer === 'server' && !isSsrTargetWebworkerEnvironment + ? nodeLikeBuiltins + : []), }, resolve ?? {}, ) @@ -1837,6 +1842,7 @@ async function bundleConfigFile( preserveSymlinks: false, packageCache, isRequire, + builtins: nodeLikeBuiltins, })?.id } @@ -1855,7 +1861,7 @@ async function bundleConfigFile( // With the `isNodeBuiltin` check above, this check captures if the builtin is a // non-node built-in, which esbuild doesn't know how to handle. In that case, we // externalize it so the non-node runtime handles it instead. - if (isBuiltin(id)) { + if (isNodeLikeBuiltin(id)) { return { external: true } } diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index 7b95d6aa92b959..40e0459cc42e1b 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -155,7 +155,9 @@ function createIsExternal( } let isExternal = false if (id[0] !== '.' && !path.isAbsolute(id)) { - isExternal = isBuiltin(id) || isConfiguredAsExternal(id, importer) + isExternal = + isBuiltin(environment.config.resolve.builtins, id) || + isConfiguredAsExternal(id, importer) } processedIds.set(id, isExternal) return isExternal diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 1bdb2d2125d539..7b065cbb0a8ea0 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -115,7 +115,7 @@ export function esbuildDepPlugin( namespace: 'optional-peer-dep', } } - if (environment.config.consumer === 'server' && isBuiltin(resolved)) { + if (isBuiltin(environment.config.resolve.builtins, resolved)) { return } if (isExternalUrl(resolved)) { diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index d9d56025221a95..4697454db613a3 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -520,7 +520,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { if (shouldExternalize(environment, specifier, importer)) { return } - if (isBuiltin(specifier)) { + if (isBuiltin(environment.config.resolve.builtins, specifier)) { return } } diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 2306a2600a1c50..f7443f0250a6e6 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -25,6 +25,7 @@ import { isDataUrl, isExternalUrl, isInNodeModules, + isNodeLikeBuiltin, isNonDriveRelativeAbsolutePath, isObject, isOptimizable, @@ -97,9 +98,9 @@ export interface EnvironmentResolveOptions { */ external?: string[] | true /** - * @internal + * Array of strings or regular expressions that indicate what modules are builtin for the environment. */ - enableBuiltinNoExternalCheck?: boolean + builtins?: (string | RegExp)[] } export interface ResolveOptions extends EnvironmentResolveOptions { @@ -173,11 +174,8 @@ interface ResolvePluginOptions { } export interface InternalResolveOptions - extends Required>, - ResolvePluginOptions { - /** @internal this is always optional for backward compat */ - enableBuiltinNoExternalCheck?: boolean -} + extends Required, + ResolvePluginOptions {} // Defined ResolveOptions are used to overwrite the values for all environments // It is used when creating custom resolvers (for CSS, scanning, etc) @@ -422,47 +420,67 @@ export function resolvePlugin( return res } - // node built-ins. - // externalize if building for a node compatible environment, otherwise redirect to empty module - if (isBuiltin(id)) { - if (currentEnvironmentOptions.consumer === 'server') { - if ( - options.enableBuiltinNoExternalCheck && - options.noExternal === true && - // if both noExternal and external are true, noExternal will take the higher priority and bundle it. - // only if the id is explicitly listed in external, we will externalize it and skip this error. - (options.external === true || !options.external.includes(id)) - ) { - let message = `Cannot bundle Node.js built-in "${id}"` - if (importer) { - message += ` imported from "${path.relative( - process.cwd(), - importer, - )}"` - } - message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` - this.error(message) + // built-ins + // externalize if building for a server environment, otherwise redirect to an empty module + if ( + currentEnvironmentOptions.consumer === 'server' && + isBuiltin(options.builtins, id) + ) { + return options.idOnly + ? id + : { id, external: true, moduleSideEffects: false } + } else if ( + currentEnvironmentOptions.consumer === 'server' && + isNodeLikeBuiltin(id) + ) { + if (!(options.external === true || options.external.includes(id))) { + let message = `Automatically externalized node built-in module "${id}"` + if (importer) { + message += ` imported from "${path.relative( + process.cwd(), + importer, + )}"` } + message += `. Consider adding it to environments.${this.environment.name}.external if it is intended.` + this.error(message) + } - return options.idOnly - ? id - : { id, external: true, moduleSideEffects: false } - } else { - if (!asSrc) { - debug?.( - `externalized node built-in "${id}" to empty module. ` + - `(imported by: ${colors.white(colors.dim(importer))})`, - ) - } else if (isProduction) { - this.warn( - `Module "${id}" has been externalized for browser compatibility, imported by "${importer}". ` + - `See https://vite.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.`, - ) + return options.idOnly + ? id + : { id, external: true, moduleSideEffects: false } + } else if ( + currentEnvironmentOptions.consumer === 'client' && + isNodeLikeBuiltin(id) + ) { + if ( + options.noExternal === true && + // if both noExternal and external are true, noExternal will take the higher priority and bundle it. + // only if the id is explicitly listed in external, we will externalize it and skip this error. + (options.external === true || !options.external.includes(id)) + ) { + let message = `Cannot bundle built-in module "${id}"` + if (importer) { + message += ` imported from "${path.relative( + process.cwd(), + importer, + )}"` } - return isProduction - ? browserExternalId - : `${browserExternalId}:${id}` + message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` + this.error(message) + } + + if (!asSrc) { + debug?.( + `externalized node built-in "${id}" to empty module. ` + + `(imported by: ${colors.white(colors.dim(importer))})`, + ) + } else if (isProduction) { + this.warn( + `Module "${id}" has been externalized for browser compatibility, imported by "${importer}". ` + + `See https://vite.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.`, + ) } + return isProduction ? browserExternalId : `${browserExternalId}:${id}` } } @@ -720,8 +738,10 @@ export function tryNodeResolve( basedir = root } + const isModuleBuiltin = (id: string) => isBuiltin(options.builtins, id) + let selfPkg = null - if (!isBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { + if (!isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { // check if it's a self reference dep. const selfPackageData = findNearestPackageData(basedir, packageCache) selfPkg = @@ -738,7 +758,7 @@ export function tryNodeResolve( // if so, we can resolve to a special id that errors only when imported. if ( basedir !== root && // root has no peer dep - !isBuiltin(id) && + !isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id) ) { diff --git a/packages/vite/src/node/ssr/fetchModule.ts b/packages/vite/src/node/ssr/fetchModule.ts index a658fe6c3e31bd..a012167a2a1720 100644 --- a/packages/vite/src/node/ssr/fetchModule.ts +++ b/packages/vite/src/node/ssr/fetchModule.ts @@ -28,8 +28,10 @@ export async function fetchModule( importer?: string, options: FetchModuleOptions = {}, ): Promise { - // builtins should always be externalized - if (url.startsWith('data:') || isBuiltin(url)) { + if ( + url.startsWith('data:') || + isBuiltin(environment.config.resolve.builtins, url) + ) { return { externalize: url, type: 'builtin' } } @@ -60,6 +62,7 @@ export async function fetchModule( isProduction, root, packageCache: environment.config.packageCache, + builtins: environment.config.resolve.builtins, }) if (!resolved) { const err: any = new Error( diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 582a3c2c6e43ef..45b51ffc9858a1 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -102,11 +102,43 @@ const BUN_BUILTIN_NAMESPACE = 'bun:' // Some runtimes like Bun injects namespaced modules here, which is not a node builtin const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) -// TODO: Use `isBuiltin` from `node:module`, but Deno doesn't support it -export function isBuiltin(id: string): boolean { - if (process.versions.deno && id.startsWith(NPM_BUILTIN_NAMESPACE)) return true - if (process.versions.bun && id.startsWith(BUN_BUILTIN_NAMESPACE)) return true - return isNodeBuiltin(id) +const isBuiltinCache = new WeakMap< + (string | RegExp)[], + (id: string, importer?: string) => boolean +>() + +export function isBuiltin(builtins: (string | RegExp)[], id: string): boolean { + let isBuiltin = isBuiltinCache.get(builtins) + if (!isBuiltin) { + isBuiltin = createIsBuiltin(builtins) + isBuiltinCache.set(builtins, isBuiltin) + } + return isBuiltin(id) +} + +export function createIsBuiltin( + builtins: (string | RegExp)[], +): (id: string) => boolean { + const plainBuiltinsSet = new Set( + builtins.filter((builtin) => typeof builtin === 'string'), + ) + const regexBuiltins = builtins.filter( + (builtin) => typeof builtin !== 'string', + ) + + return (id) => + plainBuiltinsSet.has(id) || regexBuiltins.some((regexp) => regexp.test(id)) +} + +export const nodeLikeBuiltins = [ + ...nodeBuiltins, + new RegExp(`^${NODE_BUILTIN_NAMESPACE}`), + new RegExp(`^${NPM_BUILTIN_NAMESPACE}`), + new RegExp(`^${BUN_BUILTIN_NAMESPACE}`), +] + +export function isNodeLikeBuiltin(id: string): boolean { + return isBuiltin(nodeLikeBuiltins, id) } export function isNodeBuiltin(id: string): boolean { From 612332b9bbe8d489265aea31c9c9a712319abc51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Thu, 23 Jan 2025 23:22:47 +0900 Subject: [PATCH 36/38] fix(resolve): support resolving TS files by JS extension specifiers in JS files (#18889) --- packages/vite/src/node/optimizer/scan.ts | 35 ++++---------------- packages/vite/src/node/plugins/resolve.ts | 20 ++--------- packages/vite/src/node/utils.ts | 3 -- playground/resolve/__tests__/resolve.spec.ts | 4 +++ playground/resolve/index.html | 6 ++++ playground/resolve/ts-extension/index-js.js | 10 ++++++ 6 files changed, 29 insertions(+), 49 deletions(-) create mode 100644 playground/resolve/ts-extension/index-js.js diff --git a/packages/vite/src/node/optimizer/scan.ts b/packages/vite/src/node/optimizer/scan.ts index 5c7562c5ba8f12..55b0111d57a093 100644 --- a/packages/vite/src/node/optimizer/scan.ts +++ b/packages/vite/src/node/optimizer/scan.ts @@ -105,11 +105,6 @@ export function devToScanEnvironment( } as unknown as ScanEnvironment } -type ResolveIdOptions = Omit< - Parameters[2], - 'environment' -> - const debug = createDebugger('vite:deps') const htmlTypesRE = /\.(html|vue|svelte|astro|imba)$/ @@ -383,27 +378,19 @@ function esbuildScanPlugin( async function resolveId( id: string, importer?: string, - options?: ResolveIdOptions, ): Promise { return environment.pluginContainer.resolveId( id, importer && normalizePath(importer), - { - ...options, - scan: true, - }, + { scan: true }, ) } - const resolve = async ( - id: string, - importer?: string, - options?: ResolveIdOptions, - ) => { + const resolve = async (id: string, importer?: string) => { const key = id + (importer && path.dirname(importer)) if (seen.has(key)) { return seen.get(key) } - const resolved = await resolveId(id, importer, options) + const resolved = await resolveId(id, importer) const res = resolved?.id seen.set(key, res) return res @@ -633,18 +620,14 @@ function esbuildScanPlugin( // avoid matching windows volume filter: /^[\w@][^:]/, }, - async ({ path: id, importer, pluginData }) => { + async ({ path: id, importer }) => { if (moduleListContains(exclude, id)) { return externalUnlessEntry({ path: id }) } if (depImports[id]) { return externalUnlessEntry({ path: id }) } - const resolved = await resolve(id, importer, { - custom: { - depScan: { loader: pluginData?.htmlType?.loader }, - }, - }) + const resolved = await resolve(id, importer) if (resolved) { if (shouldExternalizeDep(resolved, id)) { return externalUnlessEntry({ path: id }) @@ -706,13 +689,9 @@ function esbuildScanPlugin( { filter: /.*/, }, - async ({ path: id, importer, pluginData }) => { + async ({ path: id, importer }) => { // use vite resolver to support urls and omitted extensions - const resolved = await resolve(id, importer, { - custom: { - depScan: { loader: pluginData?.htmlType?.loader }, - }, - }) + const resolved = await resolve(id, importer) if (resolved) { if ( shouldExternalizeDep(resolved, id) || diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index f7443f0250a6e6..6c93f41c12b381 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -29,7 +29,6 @@ import { isNonDriveRelativeAbsolutePath, isObject, isOptimizable, - isTsRequest, normalizePath, safeRealpathSync, tryStatSync, @@ -125,10 +124,7 @@ interface ResolvePluginOptions { tryPrefix?: string preferRelative?: boolean isRequire?: boolean - // #3040 - // when the importer is a ts module, - // if the specifier requests a non-existent `.js/jsx/mjs/cjs` file, - // should also try import from `.ts/tsx/mts/cts` source file as fallback. + /** @deprecated */ isFromTsImporter?: boolean // True when resolving during the scan phase to discover dependencies scan?: boolean @@ -241,18 +237,6 @@ export function resolvePlugin( } } - if (importer) { - if ( - isTsRequest(importer) || - resolveOpts.custom?.depScan?.loader?.startsWith('ts') - ) { - options.isFromTsImporter = true - } else { - const moduleLang = this.getModuleInfo(importer)?.meta.vite?.lang - options.isFromTsImporter = moduleLang && isTsRequest(`.${moduleLang}`) - } - } - let res: string | PartialResolvedId | undefined // resolve pre-bundled deps requests, these could be resolved by @@ -616,7 +600,7 @@ function tryCleanFsResolve( let res: string | undefined // If path.dirname is a valid directory, try extensions and ts resolution logic - const possibleJsToTs = options.isFromTsImporter && isPossibleTsOutput(file) + const possibleJsToTs = isPossibleTsOutput(file) if (possibleJsToTs || options.extensions.length || tryPrefix) { const dirPath = path.dirname(file) if (isDirectory(dirPath)) { diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 45b51ffc9858a1..0883f2e5da4fa7 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -327,9 +327,6 @@ export const isJSRequest = (url: string): boolean => { return false } -const knownTsRE = /\.(?:ts|mts|cts|tsx)(?:$|\?)/ -export const isTsRequest = (url: string): boolean => knownTsRE.test(url) - const importQueryRE = /(\?|&)import=?(?:&|$)/ const directRequestRE = /(\?|&)direct=?(?:&|$)/ const internalPrefixes = [ diff --git a/playground/resolve/__tests__/resolve.spec.ts b/playground/resolve/__tests__/resolve.spec.ts index ba5e6441dc7edb..d5d11f4a7b08ce 100644 --- a/playground/resolve/__tests__/resolve.spec.ts +++ b/playground/resolve/__tests__/resolve.spec.ts @@ -111,6 +111,10 @@ test('a ts module can import another ts module using its corresponding js file n expect(await page.textContent('.ts-extension')).toMatch('[success]') }) +test('a js module can import another ts module using its corresponding js file name', async () => { + expect(await page.textContent('.js-ts-extension')).toMatch('[success]') +}) + test('filename with dot', async () => { expect(await page.textContent('.dot')).toMatch('[success]') }) diff --git a/playground/resolve/index.html b/playground/resolve/index.html index f1480b0a9b3e52..1b5cd5ae76a3fd 100644 --- a/playground/resolve/index.html +++ b/playground/resolve/index.html @@ -113,6 +113,9 @@

fail

+

A js module can import TS modules using its corresponding js file name

+

fail

+

Resolve file name containing dot

fail

@@ -304,6 +307,9 @@

utf8-bom-package

import { msgMjs as tsMjsExtensionWithQueryMsg } from './ts-extension?query=1' text('.mjs-extension-with-query', tsMjsExtensionWithQueryMsg) + import { msg as jsTsExtensionMsg } from './ts-extension/index-js.js' + text('.js-ts-extension', jsTsExtensionMsg) + // filename with dot import { bar } from './util/bar.util' text('.dot', bar()) diff --git a/playground/resolve/ts-extension/index-js.js b/playground/resolve/ts-extension/index-js.js new file mode 100644 index 00000000000000..0a1d17d4b10ea1 --- /dev/null +++ b/playground/resolve/ts-extension/index-js.js @@ -0,0 +1,10 @@ +import { msg as msgJs } from './hello.js' +import { msgJsx } from './hellojsx.jsx' +import { msgTsx } from './hellotsx.js' +import { msgCjs } from './hellocjs.cjs' +import { msgMjs } from './hellomjs.mjs' + +export const msg = + msgJs && msgJsx && msgTsx && msgCjs && msgMjs + ? '[success] use .js / .jsx / .cjs / .mjs extension to import a TS modules' + : '[fail]' From 51a42c6b6a285fb1f092be5bbd2e18cd1fe2b214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Thu, 23 Jan 2025 23:23:12 +0900 Subject: [PATCH 37/38] fix: make `--force` work for all environments (#18901) --- packages/vite/src/node/cli.ts | 2 +- packages/vite/src/node/config.ts | 6 ++++++ packages/vite/src/node/optimizer/index.ts | 18 +++++++++++++++--- packages/vite/src/node/server/index.ts | 4 +--- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/vite/src/node/cli.ts b/packages/vite/src/node/cli.ts index bb86e1c6e8a34a..aeaaafb697b3b3 100644 --- a/packages/vite/src/node/cli.ts +++ b/packages/vite/src/node/cli.ts @@ -189,8 +189,8 @@ cli configLoader: options.configLoader, logLevel: options.logLevel, clearScreen: options.clearScreen, - optimizeDeps: { force: options.force }, server: cleanGlobalCLIOptions(options), + forceOptimizeDeps: options.force, }) if (!server.httpServer) { diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 1669e2b2920768..00444570cbfdea 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -548,6 +548,7 @@ export interface InlineConfig extends UserConfig { /** @experimental */ configLoader?: 'bundle' | 'runner' envFile?: false + forceOptimizeDeps?: boolean } export interface ResolvedConfig @@ -775,6 +776,7 @@ function resolveEnvironmentOptions( options: EnvironmentOptions, alias: Alias[], preserveSymlinks: boolean, + forceOptimizeDeps: boolean | undefined, logger: Logger, environmentName: string, // Backward compatibility @@ -804,6 +806,7 @@ function resolveEnvironmentOptions( optimizeDeps: resolveDepOptimizationOptions( options.optimizeDeps, resolve.preserveSymlinks, + forceOptimizeDeps, consumer, ), dev: resolveDevEnvironmentOptions( @@ -980,6 +983,7 @@ function resolveResolveOptions( function resolveDepOptimizationOptions( optimizeDeps: DepOptimizationOptions | undefined, preserveSymlinks: boolean, + forceOptimizeDeps: boolean | undefined, consumer: 'client' | 'server' | undefined, ): DepOptimizationOptions { return mergeWithDefaults( @@ -990,6 +994,7 @@ function resolveDepOptimizationOptions( esbuildOptions: { preserveSymlinks, }, + force: forceOptimizeDeps ?? configDefaults.optimizeDeps.force, }, optimizeDeps ?? {}, ) @@ -1202,6 +1207,7 @@ export async function resolveConfig( config.environments[environmentName], resolvedDefaultResolve.alias, resolvedDefaultResolve.preserveSymlinks, + inlineConfig.forceOptimizeDeps, logger, environmentName, config.experimental?.skipSsrTransform, diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index fb5c71ac13a2c0..8dcf3da90675af 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -373,24 +373,36 @@ export async function loadCachedDepOptimizationMetadata( if (cachedMetadata.lockfileHash !== getLockfileHash(environment)) { environment.logger.info( 'Re-optimizing dependencies because lockfile has changed', + { + timestamp: true, + }, ) } else if (cachedMetadata.configHash !== getConfigHash(environment)) { environment.logger.info( 'Re-optimizing dependencies because vite config has changed', + { + timestamp: true, + }, ) } else { - log?.('Hash is consistent. Skipping. Use --force to override.') + log?.( + `(${environment.name}) Hash is consistent. Skipping. Use --force to override.`, + ) // Nothing to commit or cancel as we are using the cache, we only // need to resolve the processing promise so requests can move on return cachedMetadata } } } else { - environment.logger.info('Forced re-optimization of dependencies') + environment.logger.info('Forced re-optimization of dependencies', { + timestamp: true, + }) } // Start with a fresh cache - debug?.(colors.green(`removing old cache dir ${depsCacheDir}`)) + debug?.( + `(${environment.name}) ${colors.green(`removing old cache dir ${depsCacheDir}`)}`, + ) await fsp.rm(depsCacheDir, { recursive: true, force: true }) } diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index 06eaae8ff43dcc..b3c10bccf92456 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -1164,9 +1164,7 @@ async function restartServer(server: ViteDevServer) { let inlineConfig = server.config.inlineConfig if (server._forceOptimizeOnRestart) { inlineConfig = mergeConfig(inlineConfig, { - optimizeDeps: { - force: true, - }, + forceOptimizeDeps: true, }) } From bf3e41082932f4bf7d828e18ab0346b2ac8b59c9 Mon Sep 17 00:00:00 2001 From: mato533 <35281743+mato533@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:26:23 +0900 Subject: [PATCH 38/38] feat: call Logger for plugin logs in build (#13757) Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com> --- eslint.config.js | 14 +- .../vite/src/node/__tests__/build.spec.ts | 166 +++++++++++++++++- packages/vite/src/node/build.ts | 123 ++++++++----- packages/vite/src/node/plugins/worker.ts | 6 +- 4 files changed, 251 insertions(+), 58 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 968a7a89b5e1a4..6fc2d08520070a 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -8,7 +8,6 @@ import tseslint from 'typescript-eslint' import globals from 'globals' const require = createRequire(import.meta.url) -const pkg = require('./package.json') const pkgVite = require('./packages/vite/package.json') // Some rules work better with typechecking enabled, but as enabling it is slow, @@ -51,6 +50,11 @@ export default tseslint.config( ...globals.node, }, }, + settings: { + node: { + version: '^18.0.0 || ^20.0.0 || >=22.0.0', + }, + }, plugins: { n: pluginN, 'import-x': pluginImportX, @@ -213,17 +217,9 @@ export default tseslint.config( name: 'playground/test', files: ['playground/**/__tests__/**/*.?([cm])[jt]s?(x)'], rules: { - // engine field doesn't exist in playgrounds - 'n/no-unsupported-features/es-builtins': [ - 'error', - { - version: pkg.engines.node, - }, - ], 'n/no-unsupported-features/node-builtins': [ 'error', { - version: pkg.engines.node, // ideally we would like to allow all experimental features // https://github.com/eslint-community/eslint-plugin-n/issues/199 ignores: ['fetch'], diff --git a/packages/vite/src/node/__tests__/build.spec.ts b/packages/vite/src/node/__tests__/build.spec.ts index f5dd1419f04d7c..8f4e8f5455056c 100644 --- a/packages/vite/src/node/__tests__/build.spec.ts +++ b/packages/vite/src/node/__tests__/build.spec.ts @@ -1,17 +1,27 @@ import { basename, resolve } from 'node:path' import { fileURLToPath } from 'node:url' +import { stripVTControlCharacters } from 'node:util' import colors from 'picocolors' -import { describe, expect, test, vi } from 'vitest' -import type { OutputChunk, OutputOptions, RollupOutput } from 'rollup' +import { afterEach, describe, expect, test, vi } from 'vitest' +import type { + LogLevel, + OutputChunk, + OutputOptions, + RollupLog, + RollupOptions, + RollupOutput, +} from 'rollup' import type { LibraryFormats, LibraryOptions } from '../build' import { build, createBuilder, + onRollupLog, resolveBuildOutputs, resolveLibFilename, } from '../build' import type { Logger } from '../logger' import { createLogger } from '../logger' +import { BuildEnvironment, resolveConfig } from '..' const __dirname = resolve(fileURLToPath(import.meta.url), '..') @@ -876,6 +886,158 @@ test('adjust worker build error for worker.format', async () => { expect.unreachable() }) +describe('onRollupLog', () => { + const pluginName = 'rollup-plugin-test' + const msgInfo = 'This is the INFO message.' + const msgWarn = 'This is the WARN message.' + const buildProject = async ( + level: LogLevel | 'error', + message: string | RollupLog, + logger: Logger, + options?: Pick, + ) => { + await build({ + root: resolve(__dirname, 'packages/build-project'), + logLevel: 'info', + build: { + write: false, + rollupOptions: { + ...options, + logLevel: 'debug', + }, + }, + customLogger: logger, + plugins: [ + { + name: pluginName, + resolveId(id) { + this[level](message) + if (id === 'entry.js') { + return '\0' + id + } + }, + load(id) { + if (id === '\0entry.js') { + return `export default "This is test module";` + } + }, + }, + ], + }) + } + + const callOnRollupLog = async ( + logger: Logger, + level: LogLevel, + log: RollupLog, + ) => { + const config = await resolveConfig( + { customLogger: logger }, + 'build', + 'production', + 'production', + ) + const buildEnvironment = new BuildEnvironment('client', config) + onRollupLog(level, log, buildEnvironment) + } + + afterEach(() => { + vi.restoreAllMocks() + }) + + test('Rollup logs of info should be handled by vite', async () => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'info').mockImplementation(() => {}) + + await buildProject('info', msgInfo, logger) + const logs = loggerSpy.mock.calls.map((args) => + stripVTControlCharacters(args[0]), + ) + expect(logs).contain(`[plugin ${pluginName}] ${msgInfo}`) + }) + + test('Rollup logs of warn should be handled by vite', async () => { + const logger = createLogger('silent') + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await buildProject('warn', msgWarn, logger) + const logs = loggerSpy.mock.calls.map((args) => + stripVTControlCharacters(args[0]), + ) + expect(logs).contain(`[plugin ${pluginName}] ${msgWarn}`) + }) + + test('onLog passed by user is called', async () => { + const logger = createLogger('silent') + + const onLogInfo = vi.fn((_log: RollupLog) => {}) + await buildProject('info', msgInfo, logger, { + onLog(level, log) { + if (level === 'info') { + onLogInfo(log) + } + }, + }) + expect(onLogInfo).toBeCalledWith( + expect.objectContaining({ message: `[plugin ${pluginName}] ${msgInfo}` }), + ) + }) + + test('onwarn passed by user is called', async () => { + const logger = createLogger('silent') + + const onWarn = vi.fn((_log: RollupLog) => {}) + await buildProject('warn', msgWarn, logger, { + onwarn(warning) { + onWarn(warning) + }, + }) + expect(onWarn).toBeCalledWith( + expect.objectContaining({ message: `[plugin ${pluginName}] ${msgWarn}` }), + ) + }) + + test('should throw error when warning contains UNRESOLVED_IMPORT', async () => { + const logger = createLogger() + await expect(() => + callOnRollupLog(logger, 'warn', { + code: 'UNRESOLVED_IMPORT', + message: 'test', + }), + ).rejects.toThrowError(/Rollup failed to resolve import/) + }) + + test.each([[`Unsupported expression`], [`statically analyzed`]])( + 'should ignore dynamic import warnings (%s)', + async (message: string) => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await callOnRollupLog(logger, 'warn', { + code: 'PLUGIN_WARNING', + message: message, + plugin: 'rollup-plugin-dynamic-import-variables', + }) + expect(loggerSpy).toBeCalledTimes(0) + }, + ) + + test.each([[`CIRCULAR_DEPENDENCY`], [`THIS_IS_UNDEFINED`]])( + 'should ignore some warnings (%s)', + async (code: string) => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await callOnRollupLog(logger, 'warn', { + code: code, + message: 'test message', + plugin: pluginName, + }) + expect(loggerSpy).toBeCalledTimes(0) + }, + ) +}) + /** * for each chunks in output1, if there's a chunk in output2 with the same fileName, * ensure that the chunk code is the same. if not, the chunk hash should have changed. diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index ccc953c010043c..53f979655e9885 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -5,7 +5,8 @@ import type { ExternalOption, InputOption, InternalModuleFormat, - LoggingFunction, + LogLevel, + LogOrStringHandler, ModuleFormat, OutputOptions, RollupBuild, @@ -14,6 +15,7 @@ import type { RollupOptions, RollupOutput, RollupWatcher, + WarningHandlerWithDefault, WatcherOptions, } from 'rollup' import commonjsPlugin from '@rollup/plugin-commonjs' @@ -42,6 +44,7 @@ import { arraify, asyncFlatten, copyDir, + createDebugger, displayTime, emptyDir, getPkgName, @@ -596,8 +599,8 @@ async function buildEnvironment( input, plugins, external: options.rollupOptions.external, - onwarn(warning, warn) { - onRollupWarning(warning, warn, environment) + onLog(level, log) { + onRollupLog(level, log, environment) }, } @@ -1006,70 +1009,102 @@ function clearLine() { } } -export function onRollupWarning( - warning: RollupLog, - warn: LoggingFunction, +export function onRollupLog( + level: LogLevel, + log: RollupLog, environment: BuildEnvironment, ): void { - const viteWarn: LoggingFunction = (warnLog) => { - let warning: string | RollupLog - - if (typeof warnLog === 'function') { - warning = warnLog() - } else { - warning = warnLog - } - - if (typeof warning === 'object') { - if (warning.code === 'UNRESOLVED_IMPORT') { - const id = warning.id - const exporter = warning.exporter - // throw unless it's commonjs external... - if (!id || !id.endsWith('?commonjs-external')) { - throw new Error( - `[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` + - `This is most likely unintended because it can break your application at runtime.\n` + - `If you do want to externalize this module explicitly add it to\n` + - `\`build.rollupOptions.external\``, - ) - } + const debugLogger = createDebugger('vite:build') + const viteLog: LogOrStringHandler = (logLeveling, rawLogging) => { + const logging = + typeof rawLogging === 'object' ? rawLogging : { message: rawLogging } + + if (logging.code === 'UNRESOLVED_IMPORT') { + const id = logging.id + const exporter = logging.exporter + // throw unless it's commonjs external... + if (!id || !id.endsWith('?commonjs-external')) { + throw new Error( + `[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` + + `This is most likely unintended because it can break your application at runtime.\n` + + `If you do want to externalize this module explicitly add it to\n` + + `\`build.rollupOptions.external\``, + ) } + } + if (logLeveling === 'warn') { if ( - warning.plugin === 'rollup-plugin-dynamic-import-variables' && + logging.plugin === 'rollup-plugin-dynamic-import-variables' && dynamicImportWarningIgnoreList.some((msg) => - warning.message.includes(msg), + logging.message.includes(msg), ) ) { return } - if (warningIgnoreList.includes(warning.code!)) { + if (warningIgnoreList.includes(logging.code!)) { return } + } - if (warning.code === 'PLUGIN_WARNING') { - environment.logger.warn( - `${colors.bold( - colors.yellow(`[plugin:${warning.plugin}]`), - )} ${colors.yellow(warning.message)}`, - ) + switch (logLeveling) { + case 'info': + environment.logger.info(logging.message) + return + case 'warn': + environment.logger.warn(colors.yellow(logging.message)) + return + case 'error': + environment.logger.error(colors.red(logging.message)) + return + case 'debug': + debugLogger?.(logging.message) + return + default: + logLeveling satisfies never + // fallback to info if a unknown log level is passed + environment.logger.info(logging.message) return - } } - - warn(warnLog) } clearLine() - const userOnWarn = environment.config.build.rollupOptions.onwarn - if (userOnWarn) { - userOnWarn(warning, viteWarn) + const userOnLog = environment.config.build.rollupOptions?.onLog + const userOnWarn = environment.config.build.rollupOptions?.onwarn + if (userOnLog) { + if (userOnWarn) { + const normalizedUserOnWarn = normalizeUserOnWarn(userOnWarn, viteLog) + userOnLog(level, log, normalizedUserOnWarn) + } else { + userOnLog(level, log, viteLog) + } + } else if (userOnWarn) { + const normalizedUserOnWarn = normalizeUserOnWarn(userOnWarn, viteLog) + normalizedUserOnWarn(level, log) } else { - viteWarn(warning) + viteLog(level, log) } } +function normalizeUserOnWarn( + userOnWarn: WarningHandlerWithDefault, + defaultHandler: LogOrStringHandler, +): LogOrStringHandler { + return (logLevel, logging) => { + if (logLevel === 'warn') { + userOnWarn(normalizeLog(logging), (log) => + defaultHandler('warn', typeof log === 'function' ? log() : log), + ) + } else { + defaultHandler(logLevel, logging) + } + } +} + +const normalizeLog = (log: RollupLog | string): RollupLog => + typeof log === 'string' ? { message: log } : log + export function resolveUserExternal( user: ExternalOption, id: string, diff --git a/packages/vite/src/node/plugins/worker.ts b/packages/vite/src/node/plugins/worker.ts index b7eea41aac0a6a..15dd48879b7479 100644 --- a/packages/vite/src/node/plugins/worker.ts +++ b/packages/vite/src/node/plugins/worker.ts @@ -15,7 +15,7 @@ import { BuildEnvironment, createToImportMetaURLBasedRelativeRuntime, injectEnvironmentToHooks, - onRollupWarning, + onRollupLog, toOutputFilePathInJS, } from '../build' import { cleanUrl } from '../../shared/utils' @@ -85,8 +85,8 @@ async function bundleWorkerEntry( plugins: workerEnvironment.plugins.map((p) => injectEnvironmentToHooks(workerEnvironment, p), ), - onwarn(warning, warn) { - onRollupWarning(warning, warn, workerEnvironment) + onLog(level, log) { + onRollupLog(level, log, workerEnvironment) }, preserveEntrySignatures: false, })