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: avoid optimizing non-optimizable external deps #8860

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
2 changes: 1 addition & 1 deletion packages/vite/src/node/constants.ts
Expand Up @@ -41,7 +41,7 @@ export const DEFAULT_CONFIG_FILES = [

export const JS_TYPES_RE = /\.(?:j|t)sx?$|\.mjs$/

export const OPTIMIZABLE_ENTRY_RE = /\.(?:m?js|ts)$/
export const OPTIMIZABLE_ENTRY_RE = /\.(?:(m|c)?js|ts)$/

export const SPECIAL_QUERY_RE = /[\?&](?:worker|sharedworker|raw|url)\b/

Expand Down
17 changes: 12 additions & 5 deletions packages/vite/src/node/optimizer/index.ts
Expand Up @@ -14,6 +14,7 @@ import {
emptyDir,
flattenId,
getHash,
isOptimizable,
lookupFile,
normalizeId,
normalizePath,
Expand Down Expand Up @@ -647,20 +648,26 @@ export async function findKnownImports(
async function addManuallyIncludedOptimizeDeps(
deps: Record<string, string>,
config: ResolvedConfig,
extra?: string[],
extra: string[] = [],
filter?: (id: string) => boolean
): Promise<void> {
const include = [...(config.optimizeDeps?.include ?? []), ...(extra ?? [])]
if (include) {
const optimizeDepsInclude = config.optimizeDeps?.include ?? []
if (optimizeDepsInclude.length || extra.length) {
const resolve = config.createResolver({ asSrc: false, scan: true })
for (const id of include) {
for (const id of [...optimizeDepsInclude, ...extra]) {
// normalize 'foo >bar` as 'foo > bar' to prevent same id being added
// and for pretty printing
const normalizedId = normalizeId(id)
if (!deps[normalizedId] && filter?.(normalizedId) !== false) {
const entry = await resolve(id)
if (entry) {
deps[normalizedId] = entry
if (isOptimizable(entry, config.optimizeDeps)) {
deps[normalizedId] = entry
} else if (optimizeDepsInclude.includes(id)) {
config.logger.warn(
`Cannot optimize included dependency: ${colors.cyan(id)}`
)
}
} else {
throw new Error(
`Failed to resolve force included dependency: ${colors.cyan(id)}`
Expand Down
20 changes: 8 additions & 12 deletions packages/vite/src/node/optimizer/scan.ts
Expand Up @@ -6,18 +6,14 @@ import type { Loader, OnLoadResult, Plugin } from 'esbuild'
import { build, transform } from 'esbuild'
import colors from 'picocolors'
import type { ResolvedConfig } from '..'
import {
JS_TYPES_RE,
KNOWN_ASSET_TYPES,
OPTIMIZABLE_ENTRY_RE,
SPECIAL_QUERY_RE
} from '../constants'
import { JS_TYPES_RE, KNOWN_ASSET_TYPES, SPECIAL_QUERY_RE } from '../constants'
import {
cleanUrl,
createDebugger,
dataUrlRE,
externalRE,
isObject,
isOptimizable,
moduleListContains,
multilineCommentsRE,
normalizePath,
Expand Down Expand Up @@ -189,10 +185,6 @@ function esbuildScanPlugin(
'@vite/env'
]

const isOptimizable = (id: string) =>
OPTIMIZABLE_ENTRY_RE.test(id) ||
!!config.optimizeDeps.extensions?.some((ext) => id.endsWith(ext))

const externalUnlessEntry = ({ path }: { path: string }) => ({
path,
external: !entries.includes(path)
Expand Down Expand Up @@ -235,7 +227,11 @@ function esbuildScanPlugin(
// It is possible for the scanner to scan html types in node_modules.
// If we can optimize this html type, skip it so it's handled by the
// bare import resolve, and recorded as optimization dep.
if (resolved.includes('node_modules') && isOptimizable(resolved)) return
if (
resolved.includes('node_modules') &&
isOptimizable(resolved, config.optimizeDeps)
)
return
return {
path: resolved,
namespace: 'html'
Expand Down Expand Up @@ -382,7 +378,7 @@ function esbuildScanPlugin(
}
if (resolved.includes('node_modules') || include?.includes(id)) {
// dependency or forced included, externalize and stop crawling
if (isOptimizable(resolved)) {
if (isOptimizable(resolved, config.optimizeDeps)) {
depImports[id] = resolved
}
return externalUnlessEntry({ path: id })
Expand Down
11 changes: 11 additions & 0 deletions packages/vite/src/node/utils.ts
Expand Up @@ -24,6 +24,7 @@ import {
DEFAULT_EXTENSIONS,
ENV_PUBLIC_PATH,
FS_PREFIX,
OPTIMIZABLE_ENTRY_RE,
VALID_ID_PREFIX,
wildcardHosts
} from './constants'
Expand Down Expand Up @@ -91,6 +92,16 @@ export function moduleListContains(
return moduleList?.some((m) => m === id || id.startsWith(m + '/'))
}

export function isOptimizable(
id: string,
optimizeDepsConfig: ResolvedConfig['optimizeDeps']
): boolean {
return (
OPTIMIZABLE_ENTRY_RE.test(id) ||
(optimizeDepsConfig.extensions?.some((ext) => id.endsWith(ext)) ?? false)
)
}

export const bareImportRE = /^[\w@](?!.*:\/\/)/
export const deepImportRE = /^([^@][^/]*)\/|^(@[^/]+\/[^/]+)\//

Expand Down
4 changes: 4 additions & 0 deletions playground/optimize-deps/non-optimizable-include/index.css
@@ -0,0 +1,4 @@
@font-face {
font-family: 'Not Real Sans';
src: url('./i-throw-if-you-optimize-this-file.woff') format('woff');
}
9 changes: 9 additions & 0 deletions playground/optimize-deps/non-optimizable-include/package.json
@@ -0,0 +1,9 @@
{
"name": "non-optimizable-include",
"private": true,
"type": "module",
"version": "0.0.0",
"exports": {
".": "./index.css"
}
}
7 changes: 6 additions & 1 deletion playground/optimize-deps/vite.config.js
Expand Up @@ -16,7 +16,12 @@ module.exports = {
},

optimizeDeps: {
include: ['dep-linked-include', 'nested-exclude > nested-include'],
include: [
'dep-linked-include',
'nested-exclude > nested-include',
// will throw if optimized (should log warning instead)
'non-optimizable-include'
],
exclude: ['nested-exclude'],
esbuildOptions: {
plugins: [
Expand Down
4 changes: 4 additions & 0 deletions playground/ssr-deps/no-external-css/index.css
@@ -0,0 +1,4 @@
@font-face {
font-family: 'Not Real Sans';
src: url('./i-throw-if-you-optimize-this-file.woff') format('woff');
}
9 changes: 9 additions & 0 deletions playground/ssr-deps/no-external-css/package.json
@@ -0,0 +1,9 @@
{
"name": "no-external-css",
"private": true,
"type": "module",
"version": "0.0.0",
"exports": {
".": "./index.css"
}
}
3 changes: 2 additions & 1 deletion playground/ssr-deps/package.json
Expand Up @@ -19,7 +19,8 @@
"read-file-content": "file:./read-file-content",
"require-absolute": "file:./require-absolute",
"ts-transpiled-exports": "file:./ts-transpiled-exports",
"no-external-cjs": "file:./no-external-cjs"
"no-external-cjs": "file:./no-external-cjs",
"no-external-css": "file:./no-external-css"
},
"devDependencies": {
"cross-env": "^7.0.3",
Expand Down
2 changes: 1 addition & 1 deletion playground/ssr-deps/server.js
Expand Up @@ -35,7 +35,7 @@ export async function createServer(root = process.cwd(), hmrPort) {
},
appType: 'custom',
ssr: {
noExternal: ['no-external-cjs']
noExternal: ['no-external-cjs', 'no-external-css']
}
})
// use vite's connect instance as middleware
Expand Down
14 changes: 14 additions & 0 deletions pnpm-lock.yaml

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