Skip to content

Commit f7e1d07

Browse files
authored
fix(optimizer): avoid error happening with a package with asset entrypoint (#21766)
1 parent a704abc commit f7e1d07

File tree

9 files changed

+75
-13
lines changed

9 files changed

+75
-13
lines changed

packages/vite/src/node/optimizer/rolldownDepPlugin.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,19 @@ export function rolldownDepPlugin(
7676
const esmPackageCache: PackageCache = new Map()
7777
const cjsPackageCache: PackageCache = new Map()
7878

79+
const resolveAssets = (resolved: string, kind: ImportKind) => {
80+
if (kind === 'require-call') {
81+
// here it is not set to `external: true` to convert `require` to `import`
82+
return {
83+
id: externalWithConversionNamespace + resolved,
84+
}
85+
}
86+
return {
87+
id: resolved,
88+
external: 'absolute' as const,
89+
}
90+
}
91+
7992
// default resolver which prefers ESM
8093
const _resolve = createBackCompatIdResolver(environment.getTopLevelConfig(), {
8194
asSrc: false,
@@ -106,7 +119,7 @@ export function rolldownDepPlugin(
106119
return resolver(environment, id, _importer)
107120
}
108121

109-
const resolveResult = (id: string, resolved: string) => {
122+
const resolveResult = (id: string, resolved: string, kind: ImportKind) => {
110123
if (resolved.startsWith(browserExternalId)) {
111124
return {
112125
id: browserExternalNamespace + id,
@@ -117,6 +130,9 @@ export function rolldownDepPlugin(
117130
id: optionalPeerDepNamespace + resolved,
118131
}
119132
}
133+
if (allExternalTypesReg.test(resolved)) {
134+
return resolveAssets(resolved, kind)
135+
}
120136
if (isBuiltin(environment.config.resolve.builtins, resolved)) {
121137
return
122138
}
@@ -175,17 +191,7 @@ export function rolldownDepPlugin(
175191
external: false,
176192
}
177193
}
178-
179-
if (kind === 'require-call') {
180-
// here it is not set to `external: true` to convert `require` to `import`
181-
return {
182-
id: externalWithConversionNamespace + resolved,
183-
}
184-
}
185-
return {
186-
id: resolved,
187-
external: 'absolute',
188-
}
194+
return resolveAssets(resolved, kind)
189195
}
190196
},
191197
},
@@ -241,7 +247,7 @@ export function rolldownDepPlugin(
241247
// use vite's own resolver
242248
const resolved = await resolve(id, importer, kind)
243249
if (resolved) {
244-
return resolveResult(id, resolved)
250+
return resolveResult(id, resolved, kind)
245251
}
246252
},
247253
},

playground/optimize-deps/__tests__/optimize-deps.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ test('CJS dep with css import', async () => {
166166
await expect.poll(() => getColor('.cjs-with-assets')).toBe('blue')
167167
})
168168

169+
test('CJS dep requiring dep with css main field', async () => {
170+
await expect.poll(() => getColor('.cjs-require-css-main-field')).toBe('coral')
171+
})
172+
169173
test('externalize known non-js files in optimize included dep', async () => {
170174
await expect
171175
.poll(() => page.textContent('.externalize-known-non-js'))
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "@vitejs/test-dep-cjs-css-main-field",
3+
"private": true,
4+
"version": "0.0.0",
5+
"main": "style.css"
6+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.cjs-require-css-main-field {
2+
color: coral;
3+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
require('@vitejs/test-dep-cjs-css-main-field')
2+
exports.a = 1
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "@vitejs/test-dep-cjs-require-css-main-field",
3+
"private": true,
4+
"version": "0.0.0",
5+
"main": "index.js",
6+
"dependencies": {
7+
"@vitejs/test-dep-cjs-css-main-field": "file:../dep-cjs-css-main-field"
8+
}
9+
}

playground/optimize-deps/index.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ <h2>Dep with CSS</h2>
5252
<h2>CJS Dep with CSS</h2>
5353
<div class="cjs-with-assets">This should be blue</div>
5454

55+
<h2>CJS Dep requiring dep with CSS main field</h2>
56+
<div class="cjs-require-css-main-field">This should be coral</div>
57+
5558
<h2>import * as ...</h2>
5659
<div class="import-star"></div>
5760

@@ -170,6 +173,7 @@ <h2>Import the CommonJS external package that omits the js suffix</h2>
170173
}
171174

172175
import '@vitejs/test-dep-cjs-with-assets'
176+
import '@vitejs/test-dep-cjs-require-css-main-field'
173177
import '@vitejs/test-dep-css-require'
174178
import cssModuleRequire from '@vitejs/test-dep-css-require/mod.cjs'
175179
document

playground/optimize-deps/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
"@vitejs/test-dep-cjs-with-assets": "file:./dep-cjs-with-assets",
2121
"@vitejs/test-dep-cjs-with-es-module-flag": "file:./dep-cjs-with-es-module-flag",
2222
"@vitejs/test-dep-cjs-with-external-deps": "file:./dep-cjs-with-external-deps",
23+
"@vitejs/test-dep-cjs-css-main-field": "file:./dep-cjs-css-main-field",
24+
"@vitejs/test-dep-cjs-require-css-main-field": "file:./dep-cjs-require-css-main-field",
2325
"@vitejs/test-dep-css-require": "file:./dep-css-require",
2426
"@vitejs/test-dep-esbuild-plugin-transform": "file:./dep-esbuild-plugin-transform",
2527
"@vitejs/test-dep-incompatible": "file:./dep-incompatible",

pnpm-lock.yaml

Lines changed: 26 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)