diff --git a/packages/vite/src/module-runner/runner.ts b/packages/vite/src/module-runner/runner.ts index 8305fd6fd82cca..235724e7fbe6cb 100644 --- a/packages/vite/src/module-runner/runner.ts +++ b/packages/vite/src/module-runner/runner.ts @@ -144,17 +144,17 @@ export class ModuleRunner { visited.add(mod.id) for (const importedModuleId of mod.imports) { + const importedModule = + this.evaluatedModules.getModuleById(importedModuleId) + if (!importedModule?.promise || importedModule.evaluated) { + continue + } + if (callstack.includes(importedModuleId)) { return true } - const importedModule = - this.evaluatedModules.getModuleById(importedModuleId) - if ( - importedModule?.promise && - !importedModule.evaluated && - this.isCircularRequest(importedModule, callstack, visited) - ) { + if (this.isCircularRequest(importedModule, callstack, visited)) { return true } } diff --git a/packages/vite/src/node/__tests__/plugins/css.spec.ts b/packages/vite/src/node/__tests__/plugins/css.spec.ts index b7400aa6f06dfd..da8d27417b03ce 100644 --- a/packages/vite/src/node/__tests__/plugins/css.spec.ts +++ b/packages/vite/src/node/__tests__/plugins/css.spec.ts @@ -280,6 +280,15 @@ describe('convertTargets', () => { test('supports es6 as an alias of es2015', () => { expect(convertTargets('es6')).toStrictEqual(convertTargets('es2015')) }) + + test('returns undefined when there is no constraint', () => { + expect(convertTargets('esnext')).toBeUndefined() + expect(convertTargets(['esnext'])).toBeUndefined() + expect(convertTargets(false)).toBeUndefined() + expect(convertTargets(['esnext', 'chrome148'])).toStrictEqual({ + chrome: 0x94_00_00, + }) + }) }) describe('getEmptyChunkReplacer', () => { diff --git a/packages/vite/src/node/constants.ts b/packages/vite/src/node/constants.ts index edaaa2f6b70a17..200d076f76334e 100644 --- a/packages/vite/src/node/constants.ts +++ b/packages/vite/src/node/constants.ts @@ -1,12 +1,8 @@ import path, { resolve } from 'node:path' import { fileURLToPath } from 'node:url' -import { readFileSync } from 'node:fs' +import { version } from '../../package.json' with { type: 'json' } import type { RollupPluginHooks } from './typeUtils' -const { version } = JSON.parse( - readFileSync(new URL('../../package.json', import.meta.url)).toString(), -) - export const ROLLUP_HOOKS: RollupPluginHooks[] = [ 'options', 'buildStart', diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts index a758037d494d8b..fdbb8b24c3c4c2 100644 --- a/packages/vite/src/node/plugins/css.ts +++ b/packages/vite/src/node/plugins/css.ts @@ -3616,9 +3616,10 @@ const convertTargetsCache = new Map< export const convertTargets = ( esbuildTarget: string | string[] | false, ): LightningCSSOptions['targets'] => { - if (!esbuildTarget) return {} - const cached = convertTargetsCache.get(esbuildTarget) - if (cached) return cached + if (!esbuildTarget) return undefined + if (convertTargetsCache.has(esbuildTarget)) { + return convertTargetsCache.get(esbuildTarget) + } const targets: LightningCSSOptions['targets'] = {} const entriesWithoutES = arraify(esbuildTarget).flatMap((e) => { @@ -3652,8 +3653,10 @@ export const convertTargets = ( throw new Error(`Unsupported target "${entry}"`) } - convertTargetsCache.set(esbuildTarget, targets) - return targets + // an empty object means "no browser supports anything" to lightningcss + const result = Object.keys(targets).length > 0 ? targets : undefined + convertTargetsCache.set(esbuildTarget, result) + return result } export function resolveLibCssFilename( diff --git a/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/evaluated.js b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/evaluated.js new file mode 100644 index 00000000000000..b8297744a0591e --- /dev/null +++ b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/evaluated.js @@ -0,0 +1,7 @@ +const importShared = + await globalThis.__vite_ssr_hmr_evaluated_import_race__?.importShared?.() + +export const sharedValue = importShared + ? (await import('./shared.js')).value + : undefined +export const evaluated = true diff --git a/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/shared.js b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/shared.js new file mode 100644 index 00000000000000..ff5ca535f04995 --- /dev/null +++ b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-evaluated-import-race/shared.js @@ -0,0 +1,5 @@ +import './evaluated.js' + +await globalThis.__vite_ssr_hmr_evaluated_import_race__?.wait?.() + +export const value = 'ready' diff --git a/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/a.js b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/a.js new file mode 100644 index 00000000000000..012a0872a32615 --- /dev/null +++ b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/a.js @@ -0,0 +1,6 @@ +import { b } from './b.js' + +await Promise.resolve() + +export const a = 'a' +export const getB = () => b diff --git a/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/b.js b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/b.js new file mode 100644 index 00000000000000..97960da3aa6392 --- /dev/null +++ b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/b.js @@ -0,0 +1,6 @@ +import { a } from './a.js' + +await Promise.resolve() + +export const b = 'b' +export const getA = () => a diff --git a/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/index.js b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/index.js new file mode 100644 index 00000000000000..4d82536dca3a57 --- /dev/null +++ b/packages/vite/src/node/ssr/runtime/__tests__/fixtures/tla-circular/index.js @@ -0,0 +1,4 @@ +import { a, getB } from './a.js' +import { b, getA } from './b.js' + +export { a, b, getA, getB } diff --git a/packages/vite/src/node/ssr/runtime/__tests__/server-hmr.spec.ts b/packages/vite/src/node/ssr/runtime/__tests__/server-hmr.spec.ts index d529153d8aada1..b7ff6fdd3795a3 100644 --- a/packages/vite/src/node/ssr/runtime/__tests__/server-hmr.spec.ts +++ b/packages/vite/src/node/ssr/runtime/__tests__/server-hmr.spec.ts @@ -1,4 +1,5 @@ -import { describe, expect, onTestFinished } from 'vitest' +import { assert, describe, expect, onTestFinished, vi } from 'vitest' +import { promiseWithResolvers } from '../../../../shared/utils' import { createModuleRunnerTester } from './utils' describe( @@ -26,14 +27,15 @@ describe( const fixtureC = '/fixtures/c.ts' const fixtureD = '/fixtures/d.ts' - expect(runner.hmrClient!.hotModulesMap.size).toBe(2) - expect(runner.hmrClient!.dataMap.size).toBe(2) - expect(runner.hmrClient!.ctxToListenersMap.size).toBe(2) + assert(runner.hmrClient) + expect(runner.hmrClient.hotModulesMap.size).toBe(2) + expect(runner.hmrClient.dataMap.size).toBe(2) + expect(runner.hmrClient.ctxToListenersMap.size).toBe(2) for (const fixture of [fixtureC, fixtureD]) { - expect(runner.hmrClient!.hotModulesMap.has(fixture)).toBe(true) - expect(runner.hmrClient!.dataMap.has(fixture)).toBe(true) - expect(runner.hmrClient!.ctxToListenersMap.has(fixture)).toBe(true) + expect(runner.hmrClient.hotModulesMap.has(fixture)).toBe(true) + expect(runner.hmrClient.dataMap.has(fixture)).toBe(true) + expect(runner.hmrClient.ctxToListenersMap.has(fixture)).toBe(true) } }) @@ -41,6 +43,10 @@ describe( runner, }) => { const testGlobal = globalThis as any + const sharedUrl = '/fixtures/hmr-reexport-race/shared.js' + const coreUrl = '/fixtures/hmr-reexport-race/core.js' + const entryAUrl = '/fixtures/hmr-reexport-race/entry-a.js' + const entryBUrl = '/fixtures/hmr-reexport-race/entry-b.js' testGlobal.__vite_ssr_hmr_reexport_race__ = { wait: () => Promise.resolve(), @@ -49,34 +55,22 @@ describe( delete testGlobal.__vite_ssr_hmr_reexport_race__ }) - await runner.import('/fixtures/hmr-reexport-race/entry-a.js') - await runner.import('/fixtures/hmr-reexport-race/entry-b.js') - - const sharedModule = runner.evaluatedModules.getModuleByUrl( - '/fixtures/hmr-reexport-race/shared.js', - ) - const coreModule = runner.evaluatedModules.getModuleByUrl( - '/fixtures/hmr-reexport-race/core.js', - ) - const entryAModule = runner.evaluatedModules.getModuleByUrl( - '/fixtures/hmr-reexport-race/entry-a.js', - ) - const entryBModule = runner.evaluatedModules.getModuleByUrl( - '/fixtures/hmr-reexport-race/entry-b.js', - ) - expect(sharedModule).toBeDefined() - expect(coreModule).toBeDefined() - expect(entryAModule).toBeDefined() - expect(entryBModule).toBeDefined() - - let waitStarted!: () => void - const waitStartedPromise = new Promise((resolve) => { - waitStarted = resolve - }) - let releaseWait!: () => void - const waitPromise = new Promise((resolve) => { - releaseWait = resolve - }) + await runner.import(entryAUrl) + await runner.import(entryBUrl) + + const sharedModule = runner.evaluatedModules.getModuleByUrl(sharedUrl) + const coreModule = runner.evaluatedModules.getModuleByUrl(coreUrl) + const entryAModule = runner.evaluatedModules.getModuleByUrl(entryAUrl) + const entryBModule = runner.evaluatedModules.getModuleByUrl(entryBUrl) + assert(sharedModule) + assert(coreModule) + assert(entryAModule) + assert(entryBModule) + + const { promise: waitStartedPromise, resolve: waitStarted } = + promiseWithResolvers() + const { promise: waitPromise, resolve: releaseWait } = + promiseWithResolvers() testGlobal.__vite_ssr_hmr_reexport_race__ = { wait: () => { @@ -86,18 +80,18 @@ describe( } for (const module of [ - entryAModule!, - entryBModule!, - sharedModule!, - coreModule!, + entryAModule, + entryBModule, + sharedModule, + coreModule, ]) { runner.evaluatedModules.invalidateModule(module) } - const importA = runner.import('/fixtures/hmr-reexport-race/entry-a.js') + const importA = runner.import(entryAUrl) await waitStartedPromise - const importB = runner.import('/fixtures/hmr-reexport-race/entry-b.js') + const importB = runner.import(entryBUrl) // Wait deterministically until entry-b has reached the point where it // observes shared as in-flight. The `mod.imports.add(depMod.id)` line // in `request()` runs synchronously immediately before `cachedRequest` @@ -105,12 +99,7 @@ describe( // the buggy/fixed branch has either just run or is about to run on // the same microtask. `imports` is cleared by invalidateModule, so // this is a fresh signal (unlike `importers`, which is preserved). - const entryBNode = runner.evaluatedModules.getModuleByUrl( - '/fixtures/hmr-reexport-race/entry-b.js', - )! - while (!entryBNode.imports.has(sharedModule!.id)) { - await new Promise((resolve) => setImmediate(resolve)) - } + await vi.waitUntil(() => entryBModule.imports.has(sharedModule.id)) releaseWait() const results = await Promise.allSettled([importA, importB] as const) @@ -125,13 +114,97 @@ describe( }, ]) - const hmrListeners = runner.hmrClient!.hotModulesMap - expect(hmrListeners.has('/fixtures/hmr-reexport-race/entry-a.js')).toBe( - true, - ) - expect(hmrListeners.has('/fixtures/hmr-reexport-race/entry-b.js')).toBe( - true, - ) + assert(runner.hmrClient) + const hmrListeners = runner.hmrClient.hotModulesMap + expect(hmrListeners.has(entryAUrl)).toBe(true) + expect(hmrListeners.has(entryBUrl)).toBe(true) + }) + + it('does not treat evaluated imports as live circular requests', async ({ + runner, + }) => { + const testGlobal = globalThis as any + const sharedUrl = '/fixtures/hmr-evaluated-import-race/shared.js' + const evaluatedUrl = '/fixtures/hmr-evaluated-import-race/evaluated.js' + + testGlobal.__vite_ssr_hmr_evaluated_import_race__ = { + wait: () => Promise.resolve(), + importShared: () => false, + } + onTestFinished(() => { + delete testGlobal.__vite_ssr_hmr_evaluated_import_race__ + }) + + await runner.import(sharedUrl) + + const sharedModule = runner.evaluatedModules.getModuleByUrl(sharedUrl) + const evaluatedModule = + runner.evaluatedModules.getModuleByUrl(evaluatedUrl) + assert(sharedModule) + assert(evaluatedModule) + + const { promise: waitStartedPromise, resolve: waitStarted } = + promiseWithResolvers() + const { promise: waitPromise, resolve: releaseWait } = + promiseWithResolvers() + const { + promise: evaluatedWaitStartedPromise, + resolve: evaluatedWaitStarted, + } = promiseWithResolvers() + const { promise: evaluatedWaitPromise, resolve: releaseEvaluatedWait } = + promiseWithResolvers() + + let evaluatedRequestCount = 0 + testGlobal.__vite_ssr_hmr_evaluated_import_race__ = { + wait: () => { + waitStarted() + return waitPromise + }, + importShared: async () => { + if (evaluatedRequestCount++ === 0) { + evaluatedWaitStarted() + await evaluatedWaitPromise + return true + } + return false + }, + } + + runner.evaluatedModules.invalidateModule(sharedModule) + + const sharedRequest = runner.import(sharedUrl) + await waitStartedPromise + expect(sharedModule.imports.has(evaluatedModule.id)).toBe(true) + + // Start an evaluation that pauses before dynamically importing shared. + runner.evaluatedModules.invalidateModule(evaluatedModule) + + let staleEvaluatedRequestSettled = false + const staleEvaluatedRequest = runner + .import(evaluatedUrl) + .then((result) => { + staleEvaluatedRequestSettled = true + return result + }) + await evaluatedWaitStartedPromise + + // Simulate HMR restarting evaluated while the older evaluation is still + // paused. The newer evaluation completes without importing shared. + runner.evaluatedModules.invalidateModule(evaluatedModule) + await runner.import(evaluatedUrl) + expect(evaluatedModule.evaluated).toBe(true) + + releaseEvaluatedWait() + await vi.waitUntil(() => evaluatedModule.imports.has(sharedModule.id)) + expect(staleEvaluatedRequestSettled).toBe(false) + + releaseWait() + const [sharedResult, evaluatedResult] = await Promise.all([ + sharedRequest, + staleEvaluatedRequest, + ]) + expect(sharedResult.value).toBe('ready') + expect(evaluatedResult.sharedValue).toBe('ready') }) }, process.env.CI ? 50_00 : 5_000, diff --git a/packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts b/packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts index 292868d608bb71..3fe174054a1aae 100644 --- a/packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts +++ b/packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts @@ -272,6 +272,17 @@ describe('module runner initialization', async () => { expect(action).toBeDefined() }) + it('resolves circular imports between modules with top-level await', async ({ + runner, + }) => { + const mod = await runner.import('/fixtures/tla-circular/index.js') + + expect(mod.a).toBe('a') + expect(mod.b).toBe('b') + expect(mod.getA()).toBe('a') + expect(mod.getB()).toBe('b') + }) + it('this of the exported function should be undefined', async ({ runner, }) => {