Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: respect rollupOptions.external for transitive dependencies #8679

Merged
merged 6 commits into from Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 82 additions & 0 deletions packages/vite/src/node/config.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -528,6 +529,7 @@ export async function resolveConfig(

const middlewareMode = config?.server?.middlewareMode

config = mergeConfig(config, externalConfigCompat(config, configEnv))
const optimizeDeps = config.optimizeDeps || {}

const resolved: ResolvedConfig = {
Expand Down Expand Up @@ -966,3 +968,83 @@ export function isDepsOptimizerEnabled(config: ResolvedConfig): boolean {
(command === 'serve' && optimizeDeps.disabled === 'dev')
)
}

// 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
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: maybe it can be added globally/unconditionally?
esbuildCjsExternalPlugin(normalizedExternal as string[])
]
}
}
}

return additionalConfig
}
13 changes: 13 additions & 0 deletions playground/external/__tests__/external.spec.ts
@@ -0,0 +1,13 @@
import { isBuild, page } from '~utils'

describe.runIf(isBuild)('build', () => {
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')
})
})
3 changes: 3 additions & 0 deletions playground/external/dep-that-imports-vue/index.js
@@ -0,0 +1,3 @@
import { version } from 'vue'

document.querySelector('#imported-vue-version').textContent = version
7 changes: 7 additions & 0 deletions playground/external/dep-that-imports-vue/package.json
@@ -0,0 +1,7 @@
{
"name": "@vitejs/dep-that-imports-vue",
"version": "0.0.1",
"dependencies": {
"vue": "^3.2.37"
}
}
3 changes: 3 additions & 0 deletions playground/external/dep-that-requires-vue/index.js
@@ -0,0 +1,3 @@
const { version } = require('vue')

document.querySelector('#required-vue-version').textContent = version
7 changes: 7 additions & 0 deletions 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"
}
}
20 changes: 20 additions & 0 deletions playground/external/index.html
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite App</title>
<script type="importmap">
{
"imports": {
"vue": "https://unpkg.com/vue@3.2.0/dist/vue.runtime.esm-browser.js"
}
}
</script>
</head>
<body>
<p>Imported Vue version: <span id="imported-vue-version"></span></p>
<p>Required Vue version: <span id="required-vue-version"></span></p>
<script type="module" src="/src/main.js"></script>
</body>
</html>
19 changes: 19 additions & 0 deletions playground/external/package.json
@@ -0,0 +1,19 @@
{
"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": {
"@vitejs/dep-that-imports-vue": "file:./dep-that-imports-vue",
"@vitejs/dep-that-requires-vue": "file:./dep-that-requires-vue"
},
"devDependencies": {
"vite": "workspace:*",
"vue": "^3.2.37"
}
}
2 changes: 2 additions & 0 deletions playground/external/src/main.js
@@ -0,0 +1,2 @@
import '@vitejs/dep-that-imports-vue'
import '@vitejs/dep-that-requires-vue'
10 changes: 10 additions & 0 deletions playground/external/vite.config.js
@@ -0,0 +1,10 @@
import { defineConfig } from 'vite'

export default defineConfig({
build: {
minify: false,
rollupOptions: {
external: ['vue']
}
}
})
41 changes: 41 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.