fix: don't add ?import for @vite-ignoreed import (#14851)

vitejs · Nov 2, 2023 · 42fee9e · 42fee9e
1 parent ab5bb40
commit 42fee9e
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 15 deletions.
diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts
@@ -624,12 +624,12 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
               server.warmupRequest(url, { ssr })
             }
           } else if (!importer.startsWith(withTrailingSlash(clientDir))) {
+            // check @vite-ignore which suppresses dynamic import warning
+            const hasViteIgnore = hasViteIgnoreRE.test(
+              // complete expression inside parens
+              source.slice(dynamicIndex + 1, end),
+            )
             if (!isInNodeModules(importer)) {
-              // check @vite-ignore which suppresses dynamic import warning
-              const hasViteIgnore = hasViteIgnoreRE.test(
-                // complete expression inside parens
-                source.slice(dynamicIndex + 1, end),
-              )
               if (!hasViteIgnore) {
                 this.warn(
                   `\n` +
@@ -652,8 +652,9 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
             if (!ssr) {
               const url = rawUrl.replace(cleanUpRawUrlRE, '').trim()
               if (
-                !urlIsStringRE.test(url) ||
-                isExplicitImportRequired(url.slice(1, -1))
+                !hasViteIgnore &&
+                (!urlIsStringRE.test(url) ||
+                  isExplicitImportRequired(url.slice(1, -1)))
               ) {
                 needQueryInjectHelper = true
                 str().overwrite(

diff --git a/playground/dynamic-import/__tests__/dynamic-import.spec.ts b/playground/dynamic-import/__tests__/dynamic-import.spec.ts
@@ -1,5 +1,12 @@
 import { expect, test } from 'vitest'
-import { getColor, isBuild, page, serverLogs, untilUpdated } from '~utils'
+import {
+  getColor,
+  isBuild,
+  isServe,
+  page,
+  serverLogs,
+  untilUpdated,
+} from '~utils'
 
 test('should load literal dynamic import', async () => {
   await page.click('.baz')
@@ -30,11 +37,14 @@ test('should have same reference on static and dynamic js import, .mxd', async (
   await untilUpdated(() => page.textContent('.view'), 'true', true)
 })
 
-// in this case, it is not possible to detect the correct module
-test('should have same reference on static and dynamic js import, .mxd2', async () => {
-  await page.click('.mxd2')
-  await untilUpdated(() => page.textContent('.view'), 'false', true)
-})
+// in this case, it is not possible to detect the correct module in build
+test.runIf(isServe)(
+  'should have same reference on static and dynamic js import, .mxd2',
+  async () => {
+    await page.click('.mxd2')
+    await untilUpdated(() => page.textContent('.view'), 'true')
+  },
+)
 
 test('should have same reference on static and dynamic js import, .mxdjson', async () => {
   await page.click('.mxdjson')

diff --git a/playground/dynamic-import/nested/index.js b/playground/dynamic-import/nested/index.js
@@ -33,8 +33,6 @@ document.querySelector('.mxd').addEventListener('click', async () => {
 
 document.querySelector('.mxd2').addEventListener('click', async () => {
   const test = { jss: '../files/mxd.js' }
-  const ttest = test
-  const view = 'mxd'
   const { default: mxdDynamic } = await import(/*@vite-ignore*/ test.jss)
   text('.view', mxdStatic === mxdDynamic)
 })