Skip to content

Commit

Permalink
fix: ensure user plugin's resolvedId() 'meta' reply isn't lost
Browse files Browse the repository at this point in the history
This commit improves the parity of behaviour between 'vite build' and
'vite dev' when Rollup plugins retuning non-null 'meta' properties
from resolveId() hooks are used.

If one takes the Rollup behaviour to be any kind of reference in these
cases, this is fixing a small bug in Vite, which was in a very large
part already fixed by PR vitejs#5465.

Here's an explanation of the faulty behaviour:

The normalizeUrl() helper calls user plugins's resolveId() twice.

- Once with the URL string containing a query suffix.  Here it
  _ignores_ the meta reply.

- Again, with the URL no longer containing said suffix.  Here it
  doesn't ignore the meta reply.  It is stored in the moduleGraph
  node.

As can be seen, if the user's plugin meta reply depends on the query
suffix being passed in the imported URL, that meta reply won't be
registered correctly in the module graph and is not retrievable via
getModuleInfo() later on.

This change makes it so that the first call doesn't ignore the meta
reply. This makes Vite's dev server match Rollup's plugin-calling behaviour
and fixes vitejs#6766.

Fix: vitejs#6766

* packages/vite/src/node/importGlob.ts (ResolvedUrl): Import it.
(transformImportGlob): Use it.

* packages/vite/src/node/plugins/importAnalysis.ts
(ResolvedUrl): Import it.
(normalizeUrl): Use it. Always return meta.
(transform): Ensure moduleGraph is correctly built.
  • Loading branch information
joaotavora committed Feb 16, 2022
1 parent 6ba6cc6 commit 86e444e
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
3 changes: 2 additions & 1 deletion packages/vite/src/node/importGlob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from './plugins/importAnalysisBuild'
import { cleanUrl } from './utils'
import type { RollupError } from 'rollup'
import type { ResolvedUrl } from './server/moduleGraph'

export interface AssertOptions {
assert?: {
Expand All @@ -22,7 +23,7 @@ export async function transformImportGlob(
importer: string,
importIndex: number,
root: string,
normalizeUrl?: (url: string, pos: number) => Promise<[string, string]>,
normalizeUrl?: (url: string, pos: number) => Promise<ResolvedUrl>,
preload = true
): Promise<{
importsString: string
Expand Down
24 changes: 19 additions & 5 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { makeLegalIdentifier } from '@rollup/pluginutils'
import { shouldExternalizeForSSR } from '../ssr/ssrExternal'
import { performance } from 'perf_hooks'
import { transformRequest } from '../server/transformRequest'
import type { ResolvedUrl } from '../server/moduleGraph'

const isDebug = !!process.env.DEBUG
const debug = createDebugger('vite:import-analysis')
Expand Down Expand Up @@ -177,7 +178,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
const normalizeUrl = async (
url: string,
pos: number
): Promise<[string, string]> => {
): Promise<ResolvedUrl> => {
if (base !== '/' && url.startsWith(base)) {
url = url.replace(base, '/')
}
Expand Down Expand Up @@ -228,7 +229,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
}

if (isExternalUrl(url)) {
return [url, url]
return [url, url, resolved.meta]
}

// if the resolved id is not a valid browser import specifier,
Expand Down Expand Up @@ -274,7 +275,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
url = base + url.replace(/^\//, '')
}

return [url, resolved.id]
return [url, resolved.id, resolved.meta]
}

for (let index = 0; index < imports.length; index++) {
Expand Down Expand Up @@ -392,17 +393,30 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
}

// normalize
const [normalizedUrl, resolvedId] = await normalizeUrl(
const [normalizedUrl, resolvedId, resolvedMeta] = await normalizeUrl(
specifier,
start
)
let url = normalizedUrl

// record as safe modules
server?.moduleGraph.safeModulesPath.add(
moduleGraph.safeModulesPath.add(
cleanUrl(url).slice(4 /* '/@fs'.length */)
)

// ensure module is in the graph under the correct
// resolvedId and sporting correct meta properties.
const mod = moduleGraph.getModuleById(resolvedId)
if (mod) {
mod.meta = { ...mod.meta, ...resolvedMeta }
} else {
moduleGraph.ensureEntryFromResolved([
normalizedUrl,
resolvedId,
resolvedMeta
])
}

// rewrite
if (url !== specifier) {
// for optimized cjs deps, support named imports by rewriting named
Expand Down

0 comments on commit 86e444e

Please sign in to comment.