fix: data missing from moduleInfo.meta by aleclarson · Pull Request #7477 · vitejs/vite

aleclarson · 2022-03-26T16:19:53Z

Follow-up to #6811

aleclarson · 2022-04-28T02:13:18Z

packages/vite/src/node/plugins/importAnalysis.ts

+          if (mod) {
+            mod.meta = { ...mod.meta, ...resolvedMeta }
+          } else {
+            moduleGraph.ensureEntryFromResolved([


This isn't a viable solution, because we want to avoid adding a ModuleNode to the module graph until it's been successfully loaded.

This comment is evidence of that desire:

vite/packages/vite/src/node/server/transformRequest.ts

Lines 208 to 210 in 7f535ac

// ensure module in graph after successful load

const mod = await moduleGraph.ensureEntryFromUrl(url, ssr)

ensureWatchedFile(watcher, mod.file, root)

Well, we could always delete the ModuleNode if it fails to load.. :)

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. Fixes vitejs#6766

…which still has the timestamp query.

aleclarson

I did some variable name improvements on the way. LMK what you think!

aleclarson · 2022-11-16T22:20:08Z

packages/vite/src/node/plugins/css.ts

                      ),
-                      ssr
+                      ssr,
+                      false


refactor: The default value of isSelfAccepting is now undefined, so we need to pass an explicit false value wherever nothing was previously passed.

aleclarson · 2022-11-16T22:20:40Z

packages/vite/src/node/server/index.ts

    const mod = moduleGraph.urlToModuleMap.get(path)
    if (mod && mod.isSelfAccepting && mod.lastHMRTimestamp > 0) {
-      const file = getShortName(mod.file!, config.root)
+      const file = getShortName(mod.file, config.root)


refactor: Removing nullability of ModuleNode.file as discussed with @patak-dev

aleclarson · 2022-11-16T22:23:12Z

packages/vite/src/node/server/moduleGraph.ts

      const dep =
        typeof imported === 'string'
-          ? await this.ensureEntryFromUrl(imported, ssr)
+          ? this.urlToModuleMap.get(imported)!


Anything in importedModules has a module node at this point, thanks to the importAnalysis plugin.

aleclarson · 2022-11-16T22:29:29Z

packages/vite/src/node/server/moduleGraph.ts

-  ): Promise<ModuleNode> {
-    const [url, resolvedId, meta] = await this.resolveUrl(rawUrl, ssr)
-    let mod = this.idToModuleMap.get(resolvedId)
+  ensureEntryFromResolved(


A new method that allows adding a module node without calling resolveUrl again.

aleclarson · 2022-11-16T22:31:25Z

packages/vite/src/node/server/moduleGraph.ts

+    const unhashedId = removeDepVersionQuery(resolvedId)
+    if (this.idToModuleMap.has(unhashedId)) {
+      resolvedId = unhashedId


We have to strip the version query here and see if a module node exists without, as we can't safely remove it from inside ensureEntryFromUrl.

aleclarson · 2022-11-16T22:35:24Z

packages/vite/src/node/plugins/importAnalysis.ts


 export function isExplicitImportRequired(url: string): boolean {
-  return !isJSRequest(cleanUrl(url)) && !isCSSRequest(url)
+  return !isJSRequest(url) && !isCSSRequest(url)


I'm not quite sure how safe it is to change this, but it didn't break any tests (in fact, it fixed a couple tests that were broken by this PR). For example, when a .html?vue file has &lang.js at the end, we don't want to inject ?import because that leads to duplicate entries in the module graph. Let me know if this is a problematic solution.

aleclarson · 2022-11-16T22:40:01Z

packages/vite/src/node/plugins/importAnalysis.ts

+        const isPreBundled = resolvedId.startsWith(
+          getDepsCacheDirPrefix(config)
+        )
+        const isFileInsideRoot =
+          !isPreBundled && resolvedId.startsWith(root + '/')
+        const isFileOutsideRoot =
+          !isPreBundled &&
+          !isFileInsideRoot &&
+          fs.existsSync(cleanUrl(resolvedId))


style: I hoisted these conditions to improve readability.

aleclarson · 2022-11-16T22:41:41Z

packages/vite/src/node/plugins/importAnalysis.ts

        // e.g. `import 'foo'` -> `import '/@fs/.../node_modules/foo/index.js'`
-        if (resolved.id.startsWith(root + '/')) {
+        let resolvedUrl: string
+        if (isPreBundled || isFileOutsideRoot) {


I swapped the conditions so that isPreBundled || isFileOutsideRoot is checked before isFileInsideRoot, since it was possible (even common!) for an optimized dependency file to exist within the project root.

aleclarson · 2022-11-16T22:47:42Z

packages/vite/src/node/plugins/importAnalysis.ts

-              unwrapId(url),
-              ssr,
-              canSkipImportAnalysis(url)
+            const depModule = moduleGraph.ensureEntryFromResolved(


Replaced ensureEntryFromUrl call with ensureEntryFromResolved to avoid the extra resolveUrl call

aleclarson · 2022-11-16T22:56:13Z

packages/vite/src/node/plugins/importAnalysis.ts

            }
          }

+          if (isExternalUrl(normalizedUrl) || isDataUrl(normalizedUrl)) {


Avoiding adding external URLs and data URLs to the module graph.

[no ci]

aleclarson · 2022-11-16T23:06:00Z

packages/vite/src/node/plugins/importAnalysis.ts

          }
+
+          // the module graph expects a url without timestamp query
+          let hmrUrl = removeTimestampQuery(stripBase(originalUrl, base))


Have to remove ?t= to avoid duplicate module graph entry, but can't overwrite originalUrl since we need that to know if the URL was changed.

bluwy · 2023-09-19T05:34:31Z

@aleclarson There's a lot of changes unrelated to the original fix which makes it hard to review. Are you able to rebase the changes and apply the fix for #6766 specifically? Or feel free to close this too and we can continue tracking progress in that issue.

patak-cat · 2025-02-07T08:41:23Z

this should be fixed by the Environment API refactoring in Vite 6. If some of these issues persist, let's create smaller PRs to deal with them.

magne4000 · 2025-04-03T15:21:02Z

This still seems to be an issue even with the Environment API: #19674

aleclarson changed the title ~~test: add plugin test 'preserves metadata from import query string'~~ fix: data missing from moduleInfo.meta Mar 26, 2022

aleclarson mentioned this pull request Mar 26, 2022

fix: fix rollup plugin compatibility when getModuleInfo() is used #6811

Closed

9 tasks

aleclarson commented Apr 28, 2022

View reviewed changes

aleclarson mentioned this pull request Aug 20, 2022

feat: watch public files that are imported by JavaScript #9770

Closed

4 tasks

aleclarson mentioned this pull request Oct 20, 2022

feat(plugin-react): let jsxImportSource be defined with moduleInfo.meta #10552

Closed

9 tasks

aleclarson force-pushed the fix/meta-getModuleInfo branch 2 times, most recently from 6888373 to 4f2470b Compare October 20, 2022 16:31

aleclarson added 3 commits November 15, 2022 21:32

fix: only reuse ModuleGraph entry if metadata are deeply equal

5f2bfce

test: fix some broken tests

a75bf4e

aleclarson force-pushed the fix/meta-getModuleInfo branch from 4f2470b to a75bf4e Compare November 16, 2022 02:33

aleclarson added 4 commits November 15, 2022 22:25

fix(import-analysis): remove timestamp query from module specifiers

8307b3a

refactor: better name for ModuleNode constructor 2nd argument

a2fead6

fix: compare url to rawSpecifier

75be95f

…which still has the timestamp query.

wip

f63979b

aleclarson force-pushed the fix/meta-getModuleInfo branch from 9ac5e60 to f63979b Compare November 16, 2022 22:17

aleclarson added 3 commits November 16, 2022 17:37

wip

6df09d0

wip

c8acbea

wip

a89374c

aleclarson commented Nov 16, 2022

View reviewed changes

aleclarson mentioned this pull request Nov 16, 2022

chore: setIsSelfAccepting param to isSelfAccepting #10960

Closed

4 tasks

wip

41cbca0

[no ci]

aleclarson commented Nov 16, 2022

View reviewed changes

patak-cat mentioned this pull request Mar 29, 2023

perf: shorcircuit resolve in ensure entry from url #12655

Merged

4 tasks

ef4 mentioned this pull request Dec 8, 2024

Rollup-Incompatible getModuleInfo behavior #18914

Open

7 tasks

patak-cat closed this Feb 7, 2025

	// ensure module in graph after successful load
	const mod = await moduleGraph.ensureEntryFromUrl(url, ssr)
	ensureWatchedFile(watcher, mod.file, root)

Uh oh!

Conversation

aleclarson commented Mar 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aleclarson Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleclarson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bluwy commented Sep 19, 2023

Uh oh!

patak-cat commented Feb 7, 2025

Uh oh!

magne4000 commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aleclarson commented Mar 26, 2022 •

edited

Loading

aleclarson Apr 28, 2022 •

edited

Loading