From 2fea4395dcccbe1b9b91644b1017dce92671b83d Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Mon, 20 Jun 2022 20:23:50 +0800 Subject: [PATCH 1/6] test: add a failing test case that shows `rollupOptions.external` currently doesn't work for transitive dependencies It worked in Vite 2.x, though. --- .../external/__tests__/external.spec.ts | 8 ++++++ .../external/dep-that-imports-vue/index.js | 5 ++++ .../dep-that-imports-vue/package.json | 7 ++++++ playground/external/index.html | 19 ++++++++++++++ playground/external/package.json | 18 +++++++++++++ playground/external/src/main.js | 3 +++ playground/external/vite.config.js | 10 ++++++++ pnpm-lock.yaml | 25 +++++++++++++++++++ 8 files changed, 95 insertions(+) create mode 100644 playground/external/__tests__/external.spec.ts create mode 100644 playground/external/dep-that-imports-vue/index.js create mode 100644 playground/external/dep-that-imports-vue/package.json create mode 100644 playground/external/index.html create mode 100644 playground/external/package.json create mode 100644 playground/external/src/main.js create mode 100644 playground/external/vite.config.js diff --git a/playground/external/__tests__/external.spec.ts b/playground/external/__tests__/external.spec.ts new file mode 100644 index 00000000000000..b78f6f4ea0a160 --- /dev/null +++ b/playground/external/__tests__/external.spec.ts @@ -0,0 +1,8 @@ +import { isBuild, page } from '~utils' + +describe.runIf(isBuild)('build', () => { + test('should not include vue source code in the bundle', async () => { + // If `vue` is successfully externalized, the page should use the version from the import map + expect(await page.textContent('#imported-vue-version')).toBe('3.2.0') + }) +}) diff --git a/playground/external/dep-that-imports-vue/index.js b/playground/external/dep-that-imports-vue/index.js new file mode 100644 index 00000000000000..30fc573db4974b --- /dev/null +++ b/playground/external/dep-that-imports-vue/index.js @@ -0,0 +1,5 @@ +import { version } from 'vue' + +export default function foo() { + document.querySelector('#imported-vue-version').textContent = version +} diff --git a/playground/external/dep-that-imports-vue/package.json b/playground/external/dep-that-imports-vue/package.json new file mode 100644 index 00000000000000..81fd242ec8c0ab --- /dev/null +++ b/playground/external/dep-that-imports-vue/package.json @@ -0,0 +1,7 @@ +{ + "name": "dep-that-imports-vue", + "version": "0.0.1", + "dependencies": { + "vue": "^3.2.37" + } +} diff --git a/playground/external/index.html b/playground/external/index.html new file mode 100644 index 00000000000000..c0e5de572bc21d --- /dev/null +++ b/playground/external/index.html @@ -0,0 +1,19 @@ + + + + + + Vite App + + + +
+ + + diff --git a/playground/external/package.json b/playground/external/package.json new file mode 100644 index 00000000000000..d4eea95258be51 --- /dev/null +++ b/playground/external/package.json @@ -0,0 +1,18 @@ +{ + "name": "external-test", + "private": true, + "version": "0.0.0", + "scripts": { + "dev": "vite", + "build": "vite build", + "debug": "node --inspect-brk ../../packages/vite/bin/vite", + "preview": "vite preview" + }, + "dependencies": { + "dep-that-imports-vue": "file:./dep-that-imports-vue" + }, + "devDependencies": { + "vite": "workspace:*", + "vue": "^3.2.37" + } +} diff --git a/playground/external/src/main.js b/playground/external/src/main.js new file mode 100644 index 00000000000000..ca24a826b9baac --- /dev/null +++ b/playground/external/src/main.js @@ -0,0 +1,3 @@ +import foo from 'dep-that-imports-vue' + +foo() diff --git a/playground/external/vite.config.js b/playground/external/vite.config.js new file mode 100644 index 00000000000000..f6126b069cf49d --- /dev/null +++ b/playground/external/vite.config.js @@ -0,0 +1,10 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + build: { + minify: false, + rollupOptions: { + external: ['vue'] + } + } +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 50bf6b16a92af4..3f62f0fdb1eff5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -455,6 +455,23 @@ importers: dependencies: vue: 3.2.37 + playground/external: + specifiers: + dep-that-imports-vue: file:./dep-that-imports-vue + vite: workspace:* + vue: ^3.2.37 + dependencies: + dep-that-imports-vue: file:playground/external/dep-that-imports-vue + devDependencies: + vite: link:../../packages/vite + vue: 3.2.37 + + playground/external/dep-that-imports-vue: + specifiers: + vue: ^3.2.37 + dependencies: + vue: 3.2.37 + playground/file-delete-restore: specifiers: '@vitejs/plugin-react': workspace:* @@ -8676,6 +8693,14 @@ packages: version: 1.0.0 dev: false + file:playground/external/dep-that-imports-vue: + resolution: {directory: playground/external/dep-that-imports-vue, type: directory} + name: dep-that-imports-vue + version: 0.0.1 + dependencies: + vue: 3.2.37 + dev: false + file:playground/json/json-module: resolution: {directory: playground/json/json-module, type: directory} name: json-module From f243cdd03bde52a75cfc4c9ea1dc77ae1adde8ca Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Tue, 21 Jun 2022 11:44:50 +0800 Subject: [PATCH 2/6] refactor: use `@vitejs/` scope for locally linked packages As suggested by Alec on Discord --- playground/external/dep-that-imports-vue/package.json | 2 +- playground/external/package.json | 2 +- playground/external/src/main.js | 2 +- pnpm-lock.yaml | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/playground/external/dep-that-imports-vue/package.json b/playground/external/dep-that-imports-vue/package.json index 81fd242ec8c0ab..2c088c73cbb035 100644 --- a/playground/external/dep-that-imports-vue/package.json +++ b/playground/external/dep-that-imports-vue/package.json @@ -1,5 +1,5 @@ { - "name": "dep-that-imports-vue", + "name": "@vitejs/dep-that-imports-vue", "version": "0.0.1", "dependencies": { "vue": "^3.2.37" diff --git a/playground/external/package.json b/playground/external/package.json index d4eea95258be51..08c12b4f204966 100644 --- a/playground/external/package.json +++ b/playground/external/package.json @@ -9,7 +9,7 @@ "preview": "vite preview" }, "dependencies": { - "dep-that-imports-vue": "file:./dep-that-imports-vue" + "@vitejs/dep-that-imports-vue": "file:./dep-that-imports-vue" }, "devDependencies": { "vite": "workspace:*", diff --git a/playground/external/src/main.js b/playground/external/src/main.js index ca24a826b9baac..f464a1ef184aa4 100644 --- a/playground/external/src/main.js +++ b/playground/external/src/main.js @@ -1,3 +1,3 @@ -import foo from 'dep-that-imports-vue' +import foo from '@vitejs/dep-that-imports-vue' foo() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3f62f0fdb1eff5..be2b7effa91d38 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -457,11 +457,11 @@ importers: playground/external: specifiers: - dep-that-imports-vue: file:./dep-that-imports-vue + '@vitejs/dep-that-imports-vue': file:./dep-that-imports-vue vite: workspace:* vue: ^3.2.37 dependencies: - dep-that-imports-vue: file:playground/external/dep-that-imports-vue + '@vitejs/dep-that-imports-vue': file:playground/external/dep-that-imports-vue devDependencies: vite: link:../../packages/vite vue: 3.2.37 From b487390dc9afb9fdbf3141ff103fa5daa2046159 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Tue, 21 Jun 2022 11:46:23 +0800 Subject: [PATCH 3/6] test: simplify the test --- playground/external/dep-that-imports-vue/index.js | 4 +--- playground/external/src/main.js | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/playground/external/dep-that-imports-vue/index.js b/playground/external/dep-that-imports-vue/index.js index 30fc573db4974b..aa79957e9b3b49 100644 --- a/playground/external/dep-that-imports-vue/index.js +++ b/playground/external/dep-that-imports-vue/index.js @@ -1,5 +1,3 @@ import { version } from 'vue' -export default function foo() { - document.querySelector('#imported-vue-version').textContent = version -} +document.querySelector('#imported-vue-version').textContent = version diff --git a/playground/external/src/main.js b/playground/external/src/main.js index f464a1ef184aa4..d470fc3000876a 100644 --- a/playground/external/src/main.js +++ b/playground/external/src/main.js @@ -1,3 +1 @@ -import foo from '@vitejs/dep-that-imports-vue' - -foo() +import '@vitejs/dep-that-imports-vue' From 4c869bfe44ec07b94623be46fbac53d467b5e646 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Tue, 21 Jun 2022 11:53:01 +0800 Subject: [PATCH 4/6] test: add require test --- playground/external/__tests__/external.spec.ts | 7 ++++++- .../external/dep-that-requires-vue/index.js | 3 +++ .../dep-that-requires-vue/package.json | 7 +++++++ playground/external/index.html | 3 ++- playground/external/package.json | 3 ++- playground/external/src/main.js | 1 + pnpm-lock.yaml | 18 +++++++++++++++++- 7 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 playground/external/dep-that-requires-vue/index.js create mode 100644 playground/external/dep-that-requires-vue/package.json diff --git a/playground/external/__tests__/external.spec.ts b/playground/external/__tests__/external.spec.ts index b78f6f4ea0a160..ed1d1872181c02 100644 --- a/playground/external/__tests__/external.spec.ts +++ b/playground/external/__tests__/external.spec.ts @@ -1,8 +1,13 @@ import { isBuild, page } from '~utils' describe.runIf(isBuild)('build', () => { - test('should not include vue source code in the bundle', async () => { + test('should externalize imported packages', async () => { // If `vue` is successfully externalized, the page should use the version from the import map expect(await page.textContent('#imported-vue-version')).toBe('3.2.0') }) + + test('should externalize required packages', async () => { + // If `vue` is successfully externalized, the page should use the version from the import map + expect(await page.textContent('#required-vue-version')).toBe('3.2.0') + }) }) diff --git a/playground/external/dep-that-requires-vue/index.js b/playground/external/dep-that-requires-vue/index.js new file mode 100644 index 00000000000000..0b573021a00b57 --- /dev/null +++ b/playground/external/dep-that-requires-vue/index.js @@ -0,0 +1,3 @@ +const { version } = require('vue') + +document.querySelector('#required-vue-version').textContent = version diff --git a/playground/external/dep-that-requires-vue/package.json b/playground/external/dep-that-requires-vue/package.json new file mode 100644 index 00000000000000..a12c43b40fb282 --- /dev/null +++ b/playground/external/dep-that-requires-vue/package.json @@ -0,0 +1,7 @@ +{ + "name": "@vitejs/dep-that-requires-vue", + "version": "0.0.1", + "dependencies": { + "vue": "^3.2.37" + } +} diff --git a/playground/external/index.html b/playground/external/index.html index c0e5de572bc21d..66a16c329c1fc5 100644 --- a/playground/external/index.html +++ b/playground/external/index.html @@ -13,7 +13,8 @@ -
+

Imported Vue version:

+

Required Vue version:

diff --git a/playground/external/package.json b/playground/external/package.json index 08c12b4f204966..94fdc429ce2b0b 100644 --- a/playground/external/package.json +++ b/playground/external/package.json @@ -9,7 +9,8 @@ "preview": "vite preview" }, "dependencies": { - "@vitejs/dep-that-imports-vue": "file:./dep-that-imports-vue" + "@vitejs/dep-that-imports-vue": "file:./dep-that-imports-vue", + "@vitejs/dep-that-requires-vue": "file:./dep-that-requires-vue" }, "devDependencies": { "vite": "workspace:*", diff --git a/playground/external/src/main.js b/playground/external/src/main.js index d470fc3000876a..0e1d3bece640e9 100644 --- a/playground/external/src/main.js +++ b/playground/external/src/main.js @@ -1 +1,2 @@ import '@vitejs/dep-that-imports-vue' +import '@vitejs/dep-that-requires-vue' diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be2b7effa91d38..2df9b917f91b85 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -458,10 +458,12 @@ importers: playground/external: specifiers: '@vitejs/dep-that-imports-vue': file:./dep-that-imports-vue + '@vitejs/dep-that-requires-vue': file:./dep-that-requires-vue vite: workspace:* vue: ^3.2.37 dependencies: '@vitejs/dep-that-imports-vue': file:playground/external/dep-that-imports-vue + '@vitejs/dep-that-requires-vue': file:playground/external/dep-that-requires-vue devDependencies: vite: link:../../packages/vite vue: 3.2.37 @@ -472,6 +474,12 @@ importers: dependencies: vue: 3.2.37 + playground/external/dep-that-requires-vue: + specifiers: + vue: ^3.2.37 + dependencies: + vue: 3.2.37 + playground/file-delete-restore: specifiers: '@vitejs/plugin-react': workspace:* @@ -8695,7 +8703,15 @@ packages: file:playground/external/dep-that-imports-vue: resolution: {directory: playground/external/dep-that-imports-vue, type: directory} - name: dep-that-imports-vue + name: '@vitejs/dep-that-imports-vue' + version: 0.0.1 + dependencies: + vue: 3.2.37 + dev: false + + file:playground/external/dep-that-requires-vue: + resolution: {directory: playground/external/dep-that-requires-vue, type: directory} + name: '@vitejs/dep-that-requires-vue' version: 0.0.1 dependencies: vue: 3.2.37 From 639d363a8cbc4b4c765df1f3d4d057b70e511208 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Wed, 22 Jun 2022 15:33:54 +0800 Subject: [PATCH 5/6] fix: support the simple case for `import` external --- packages/vite/src/node/config.ts | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index d6ba20fb610d08..df77a9e7b3b379 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -528,6 +528,7 @@ export async function resolveConfig( const middlewareMode = config?.server?.middlewareMode + config = mergeConfig(config, externalConfigCompat(config, configEnv)) const optimizeDeps = config.optimizeDeps || {} const resolved: ResolvedConfig = { @@ -966,3 +967,56 @@ export function isDepsOptimizerEnabled(config: ResolvedConfig): boolean { (command === 'serve' && optimizeDeps.disabled === 'dev') ) } + +// Support `rollupOptions.external` when `legacy.buildRollupPluginCommonjs` is disabled +function externalConfigCompat(config: UserConfig, { command }: ConfigEnv) { + // Only affects the build command + if (command !== 'build') { + return {} + } + + // Skip if using Rollup CommonJS plugin + if ( + config.legacy?.buildRollupPluginCommonjs || + config.optimizeDeps?.disabled === 'build' + ) { + return {} + } + + // Skip if no `external` configured + const external = config?.build?.rollupOptions?.external + if (!external) { + return {} + } + + let normalizedExternal = external + if (typeof external === 'string') { + normalizedExternal = [external] + } + + // TODO: decide whether to support RegExp and function options + // They're not supported yet because `optimizeDeps.exclude` currently only accepts strings + if ( + !Array.isArray(normalizedExternal) || + normalizedExternal.some((ext) => typeof ext !== 'string') + ) { + throw new Error( + `[vite] 'build.rollupOptions.external' can only be an array of strings or a string.\n` + + `You can turn on 'legacy.buildRollupPluginCommonjs' to support more advanced options.` + ) + } + + const additionalConfig: UserConfig = { + optimizeDeps: { + exclude: normalizedExternal as string[], + esbuildOptions: { + plugins: [ + // TODO: configure `optimizeDeps.esbuildOptions.plugins` + // TODO: maybe it can be added unconditionally? + ] + } + } + } + + return additionalConfig +} From fd88ff93254cdcc6bb922af1d704095367d0bd2a Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Wed, 22 Jun 2022 16:21:45 +0800 Subject: [PATCH 6/6] fix: fix cjs external --- packages/vite/src/node/config.ts | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index df77a9e7b3b379..07893761039144 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -7,6 +7,7 @@ import colors from 'picocolors' import type { Alias, AliasOptions } from 'types/alias' import aliasPlugin from '@rollup/plugin-alias' import { build } from 'esbuild' +import type { Plugin as ESBuildPlugin } from 'esbuild' import type { RollupOptions } from 'rollup' import type { Plugin } from './plugin' import type { BuildOptions, ResolvedBuildOptions } from './build' @@ -968,6 +969,33 @@ export function isDepsOptimizerEnabled(config: ResolvedConfig): boolean { ) } +// esbuild doesn't transpile `require('foo')` into `import` statements if 'foo' is externalized +// https://github.com/evanw/esbuild/issues/566#issuecomment-735551834 +function esbuildCjsExternalPlugin(externals: string[]): ESBuildPlugin { + return { + name: 'cjs-external', + setup(build) { + const escape = (text: string) => + `^${text.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&')}$` + const filter = new RegExp(externals.map(escape).join('|')) + + build.onResolve({ filter: /.*/, namespace: 'external' }, (args) => ({ + path: args.path, + external: true + })) + + build.onResolve({ filter }, (args) => ({ + path: args.path, + namespace: 'external' + })) + + build.onLoad({ filter: /.*/, namespace: 'external' }, (args) => ({ + contents: `export * from ${JSON.stringify(args.path)}` + })) + } + } +} + // Support `rollupOptions.external` when `legacy.buildRollupPluginCommonjs` is disabled function externalConfigCompat(config: UserConfig, { command }: ConfigEnv) { // Only affects the build command @@ -1011,8 +1039,8 @@ function externalConfigCompat(config: UserConfig, { command }: ConfigEnv) { exclude: normalizedExternal as string[], esbuildOptions: { plugins: [ - // TODO: configure `optimizeDeps.esbuildOptions.plugins` - // TODO: maybe it can be added unconditionally? + // TODO: maybe it can be added globally/unconditionally? + esbuildCjsExternalPlugin(normalizedExternal as string[]) ] } }