Skip to content
7 changes: 4 additions & 3 deletions packages/vite-dev-server/src/node/optimizer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export type ExportsData = {

export interface DepsOptimizer {
init: () => Promise<void>
initState: 'idle' | 'initializing' | 'initialized'

metadata: DepOptimizationMetadata
scanProcessing?: Promise<void>
Expand All @@ -79,8 +80,8 @@ export interface DepsOptimizer {

export interface DepOptimizationConfig {
/**
* Force optimize listed dependencies (must be resolvable import paths,
* cannot be globs).
* Force optimize listed dependencies (must be resolvable import paths).
* Supports experimental glob patterns for deep imports.
*/
include?: string[]
/**
Expand Down Expand Up @@ -829,7 +830,7 @@ async function prepareRolldownOptimizerRun(

const plugins = await asyncFlatten(arraify(pluginsFromConfig))
if (external.length) {
plugins.push(rolldownCjsExternalPlugin(external, platform))
plugins.push(rolldownCjsExternalPlugin(external, platform, environment))
}
plugins.push(...rolldownDepPlugin(environment, flatIdDeps, external))

Expand Down
22 changes: 22 additions & 0 deletions packages/vite-dev-server/src/node/optimizer/optimizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,28 @@ describe('createDepsOptimizer initialization', () => {
await optimizer.close()
})

// regression test for https://github.com/vitejs/vite/issues/23143
it('resolves discovered dep processing promise on close before init', async () => {
const optimizer = createDepsOptimizer(createEnvironment())
const info = optimizer.registerMissingImport(
'example',
'/node_modules/example/index.js',
)
let resolved = false
void info.processing?.then(() => {
resolved = true
})

// The processing promise stays pending because init() has not run.
await vi.advanceTimersByTimeAsync(500)
expect(resolved).toBe(false)

// close() resolves it so requests waiting on the dep can unblock.
await optimizer.close()
await vi.advanceTimersByTimeAsync(0)
expect(resolved).toBe(true)
})

it('does not schedule an optimizer run while initializing', async () => {
let resolveCachedMetadata:
| ((metadata: DepOptimizationMetadata) => void)
Expand Down
31 changes: 18 additions & 13 deletions packages/vite-dev-server/src/node/optimizer/optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function createDepsOptimizer(

const depsOptimizer: DepsOptimizer = {
init,
initState: 'idle',
metadata,
registerMissingImport,
run: () => debouncedProcessing(0),
Expand Down Expand Up @@ -147,17 +148,24 @@ export function createDepsOptimizer(

async function close() {
closed = true

// Ensure that a rerun will not be issued
if (debounceProcessingHandle) { clearTimeout(debounceProcessingHandle) }
debounceProcessingHandle = undefined

await Promise.allSettled([
discover?.cancel(),
depsOptimizer.scanProcessing,
optimizationResult?.cancel(),
])

depOptimizationProcessing.resolve()
resolveEnqueuedProcessingPromises()
}

let initState: 'idle' | 'initializing' | 'initialized' = 'idle'
async function init() {
if (initState !== 'idle') { return }
initState = 'initializing'
if (depsOptimizer.initState !== 'idle') { return }
depsOptimizer.initState = 'initializing'

const cachedMetadata = await loadCachedDepOptimizationMetadata(environment)

Expand Down Expand Up @@ -283,7 +291,7 @@ export function createDepsOptimizer(
})
}
}
initState = 'initialized'
depsOptimizer.initState = 'initialized'
}

function startNextDiscoveredBatch() {
Expand Down Expand Up @@ -337,11 +345,8 @@ export function createDepsOptimizer(

if (closed) {
currentlyProcessing = false
depOptimizationProcessing.resolve()
resolveEnqueuedProcessingPromises()
return
}

currentlyProcessing = true

try {
Expand All @@ -360,7 +365,6 @@ export function createDepsOptimizer(
if (closed) {
currentlyProcessing = false
processingResult.cancel()
resolveEnqueuedProcessingPromises()
return
}

Expand Down Expand Up @@ -582,7 +586,7 @@ export function createDepsOptimizer(
// A module can be transformed between `createServer()` and `server.listen()`,
// which discovers a dep before `init()` runs. Starting a run here would race
// with `init()` resetting the metadata and crash in `commitProcessing`.
if (initState === 'initialized' && !waitingForCrawlEnd) {
if (depsOptimizer.initState === 'initialized' && !waitingForCrawlEnd) {
// Debounced rerun, let other missing dependencies be discovered before
// the running next optimizeDeps
debouncedProcessing()
Expand Down Expand Up @@ -752,8 +756,9 @@ export function createDepsOptimizer(
export function createExplicitDepsOptimizer(
environment: DevEnvironment,
): DepsOptimizer {
const depsOptimizer = {
const depsOptimizer: DepsOptimizer = {
metadata: initDepsOptimizerMetadata(environment),
initState: 'idle',
isOptimizedDepFile: createIsOptimizedDepFile(environment),
isOptimizedDepUrl: createIsOptimizedDepUrl(environment),
getOptimizedDepId: (depInfo: OptimizedDepInfo) =>
Expand All @@ -773,12 +778,12 @@ export function createExplicitDepsOptimizer(
options: environment.config.optimizeDeps,
}

let inited = false
async function init() {
if (inited) { return }
inited = true
if (depsOptimizer.initState !== 'idle') { return }
depsOptimizer.initState = 'initializing'

depsOptimizer.metadata = await optimizeExplicitEnvironmentDeps(environment)
depsOptimizer.initState = 'initialized'
}

return depsOptimizer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ vi.mock('../plugins/resolve', () => ({
}))

import type { Environment } from '../environment'
import { rolldownDepPlugin } from './rolldownDepPlugin'
import {
rolldownCjsExternalPlugin,
rolldownDepPlugin,
} from './rolldownDepPlugin'

type ResolveIdHandler = (
this: PluginContext,
Expand Down Expand Up @@ -157,6 +160,96 @@ describe('rolldownDepPlugin asset entrypoints', () => {
})
})

describe('rolldownDepPlugin browser externals', () => {
const importer = '/project/node_modules/dep/index.js'

test('treats an exact browser-external id as an explicit browser:false mapping', async () => {
const { depPlugin } = createPlugins('__vite-browser-external')

expect(
await getResolveIdHandler(depPlugin).call(context, 'shimmed', importer, {
isEntry: false,
kind: 'import-statement',
}),
).toEqual({ id: '__vite-browser-external' })
expect(
await getLoadHandler(depPlugin).call(context, '__vite-browser-external'),
).toEqual({ code: 'module.exports = {}' })
})

test('keeps the warning proxy for unsupported Node builtins', async () => {
const { depPlugin } = createPlugins('__vite-browser-external:fs')

expect(
await getResolveIdHandler(depPlugin).call(context, 'fs', importer, {
isEntry: false,
kind: 'import-statement',
}),
).toEqual({ id: 'browser-external:fs' })
expect(
await getLoadHandler(depPlugin).call(context, 'browser-external:fs'),
).toMatchObject({
code: expect.stringContaining(
'has been externalized for browser compatibility',
),
})
})
})

describe('rolldownCjsExternalPlugin', () => {
const importer = '/project/node_modules/dep/index.js'

function createCjsExternalPlugin(resolve: () => Promise<string>) {
const config = {
createResolver: () => vi.fn<() => Promise<string>>(resolve),
}
const environment = {
name: 'client',
config,
getTopLevelConfig: () => config,
} as unknown as Environment
const plugin = rolldownCjsExternalPlugin(
['optional-peer', 'excluded'],
'browser',
environment,
)
if (!plugin) {
throw new Error('Could not create the cjs-external plugin')
}
return plugin
}

test('keeps a missing optional peer require on the pre-bundler CJS stub', async () => {
const resolved = '__vite-optional-peer-dep:optional-peer:dep'
const plugin = createCjsExternalPlugin(async () => resolved)

expect(
await getResolveIdHandler(plugin).call(context, 'optional-peer', importer, {
isEntry: false,
kind: 'require-call',
}),
).toEqual({ id: 'optional-peer-dep:' + resolved })
})

test('converts other excluded requires to ESM facades', async () => {
const resolvedPlugin = createCjsExternalPlugin(
async () => '/project/node_modules/excluded/index.js',
)
const unresolvablePlugin = createCjsExternalPlugin(() =>
Promise.reject(new Error('not resolvable until served')),
)

for (const plugin of [resolvedPlugin, unresolvablePlugin]) {
expect(
await getResolveIdHandler(plugin).call(context, 'excluded', importer, {
isEntry: false,
kind: 'require-call',
}),
).toEqual({ id: 'vite:cjs-external-facadeexcluded' })
}
})
})

describe('rolldownDepPlugin asset URL transform filter', () => {
const resolved = '/project/node_modules/pkg/index.js'
const id = '/project/node_modules/pkg/index.js'
Expand Down
32 changes: 30 additions & 2 deletions packages/vite-dev-server/src/node/optimizer/rolldownDepPlugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import path from 'node:path'
import type { ImportKind, Plugin, RolldownPlugin } from 'rolldown'
import { prefixRegex } from 'rolldown/filter'
import { exactRegex, prefixRegex } from 'rolldown/filter'
import MagicString from 'magic-string'
import { stripLiteral } from 'strip-literal'
import { JS_TYPES_RE, KNOWN_ASSET_TYPES } from '../constants'
Expand Down Expand Up @@ -125,6 +125,11 @@ export function rolldownDepPlugin(
}

const resolveResult = (id: string, resolved: string, kind: ImportKind) => {
// An exact browser-external id is an explicit browser:false mapping.
// Suffixed ids are unsupported Node builtins and still need the warning.
if (resolved === browserExternalId) {
return { id: browserExternalId }
}
if (resolved.startsWith(browserExternalId)) {
return {
id: browserExternalNamespace + id,
Expand Down Expand Up @@ -260,11 +265,15 @@ export function rolldownDepPlugin(
load: {
filter: {
id: [
exactRegex(browserExternalId),
prefixRegex(browserExternalNamespace),
prefixRegex(optionalPeerDepNamespace),
],
},
handler(id) {
if (id === browserExternalId) {
return { code: 'module.exports = {}' }
}
if (id.startsWith(browserExternalNamespace)) {
const path = id.slice(browserExternalNamespace.length)
if (isProduction) {
Expand Down Expand Up @@ -384,6 +393,7 @@ const matchesEntireLine = (text: string) => `^${escapeRegex(text)}$`
export function rolldownCjsExternalPlugin(
externals: string[],
platform: 'node' | 'browser' | 'neutral',
environment: Environment,
): Plugin | undefined {
// Skip this plugin for `platform: 'node'` as `require` is available in Node
// and that is more accurate than converting to `import`
Expand All @@ -401,18 +411,36 @@ export function rolldownCjsExternalPlugin(

const filter = new RegExp(externals.map(matchesEntireLine).join('|'))

const packageCache: PackageCache = new Map()
const resolveRequire = createBackCompatIdResolver(
environment.getTopLevelConfig(),
{ asSrc: false, isRequire: true, scan: true, packageCache },
)

return {
name: 'cjs-external',
resolveId: {
filter: { id: [prefixRegex(nonFacadePrefix), filter] },
handler(id, _importer, options) {
async handler(id, importer, options) {
if (id.startsWith(nonFacadePrefix)) {
return {
id: id.slice(nonFacadePrefix.length),
external: 'absolute',
}
}
if (options.kind === 'require-call') {
// A missing optional peer must throw when require() runs, not when
// an eager ESM facade is evaluated. Reuse the pre-bundler's CJS stub
// so the dependency's try/catch can still select its fallback.
// Other excluded deps may intentionally be unresolvable until served.
const resolved = await resolveRequire(
environment,
id,
importer,
).catch(() => undefined)
if (resolved?.startsWith(optionalPeerDepId)) {
return { id: optionalPeerDepNamespace + resolved }
}
return {
id: cjsExternalFacadeNamespace + id,
}
Expand Down
Loading
Loading