Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 25 additions & 24 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,32 +373,8 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
return [resolved.id, resolved.id]
}

const isRelative = url[0] === '.'
const isSelfImport = !isRelative && cleanUrl(url) === cleanUrl(importer)

url = normalizeResolvedIdToUrl(environment, url, resolved)

// make the URL browser-valid
if (environment.config.consumer === 'client') {
// mark non-js/css imports with `?import`
if (isExplicitImportRequired(url)) {
url = injectQuery(url, 'import')
} else if (
(isRelative || isSelfImport) &&
!DEP_VERSION_RE.test(url)
) {
// If the url isn't a request for a pre-bundled common chunk,
// for relative js/css imports, or self-module virtual imports
// (e.g. vue blocks), inherit importer's version query
// do not do this for unknown type imports, otherwise the appended
// query can break 3rd party plugin's extension checks.
const versionMatch = DEP_VERSION_RE.exec(importer)
if (versionMatch) {
url = injectQuery(url, versionMatch[1])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing moduleGraph._ensureEntryFromUrl below to be called with

await moduleGraph._ensureEntryFromUrl(
    unwrapId(url), // url with `?v=` query
    canSkipImportAnalysis(url) || forceSkipImportAnalysis,
    resolved, // resolved result without `?v=` query
  )

and caused the moduleGraph.urlToModuleMap to have a mapping from "url with v query" to "id without v query".
This lead id in doTransform to resolve "url with v query" to "id without v query", and made the new condition in vite:optimized-deps plugin to fail.

const resolved = module
? undefined
: ((await pluginContainer.resolveId(url, undefined)) ?? undefined)
// resolve
const id = module?.id ?? resolved?.id ?? url

https://github.com/vitejs/vite/pull/20439/files#diff-0648e3b1da8ac697354cfca37ada638d3a353be4b3312211c6b131597f6ca101R80

To fix that, I moved this if block after the moduleGraph._ensureEntryFromUrl call.
Given that _ensureEntryFromUrl removes some query internally,

rawUrl = removeImportQuery(removeTimestampQuery(rawUrl))

I think not adding ?v= for the URL passed to _ensureEntryFromUrl should be fine.

}
}
}

try {
// delay setting `isSelfAccepting` until the file is actually used (#7870)
// We use an internal function to avoid resolving the url again
Expand All @@ -423,6 +399,31 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
throw e
}

// make the URL browser-valid
if (environment.config.consumer === 'client') {
const isRelative = url[0] === '.'
const isSelfImport =
!isRelative && cleanUrl(url) === cleanUrl(importer)

// mark non-js/css imports with `?import`
if (isExplicitImportRequired(url)) {
url = injectQuery(url, 'import')
} else if (
(isRelative || isSelfImport) &&
!DEP_VERSION_RE.test(url)
) {
// If the url isn't a request for a pre-bundled common chunk,
// for relative js/css imports, or self-module virtual imports
// (e.g. vue blocks), inherit importer's version query
// do not do this for unknown type imports, otherwise the appended
// query can break 3rd party plugin's extension checks.
const versionMatch = DEP_VERSION_RE.exec(importer)
if (versionMatch) {
url = injectQuery(url, versionMatch[1])
}
}
}

// prepend base
if (!ssr) url = joinUrlSegments(base, url)

Expand Down
7 changes: 3 additions & 4 deletions packages/vite/src/node/plugins/optimizedDeps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function optimizedDepsPlugin(): Plugin {
if (depsOptimizer?.isOptimizedDepFile(id)) {
const metadata = depsOptimizer.metadata
const file = cleanUrl(id)
const versionMatch = DEP_VERSION_RE.exec(file)
const versionMatch = DEP_VERSION_RE.exec(id)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versionMatch always returned undefined because cleanUrl strips the query.
This was a bug introduced in https://github.com/vitejs/vite/pull/17789/files#diff-0648e3b1da8ac697354cfca37ada638d3a353be4b3312211c6b131597f6ca101.

const browserHash = versionMatch
? versionMatch[1].split('=')[1]
: undefined
Expand Down Expand Up @@ -77,9 +77,8 @@ export function optimizedDepsPlugin(): Plugin {
try {
return await fsp.readFile(file, 'utf-8')
} catch {
const newMetadata = depsOptimizer.metadata
if (optimizedDepInfoFromFile(newMetadata, file)) {
// Outdated non-entry points (CHUNK), loaded after a rerun
if (browserHash) {
// Outdated optimized files loaded after a rerun
Comment on lines -80 to +81
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This old condition did not match requests for stale optimized files.
The requests that was sent between "the optimizer starts the bundle" and "the optimizer finishes the bundle" is handled above after await info.processing, but the requests that was sent after "the optimizer finishes the bundle" doesn't match the condition above.

Assuming that cases like new URL('./something', import.meta.url) does not have a ?v= query, I changed the condition to use that.

throwOutdatedRequest(id)
}
throwFileNotFoundInOptimizedDep(id)
Expand Down
Loading