Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: esbuild glob resolve error #14533

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Oct 4, 2023

Description

This PR tries to fix the scanner error in https:/vitejs/vite-ecosystem-ci/actions/runs/6401781367/job/17379560146#step:8:460 (plugin-vue).

Could not resolve import("../components/**/*.vue")

This error was happening because resolveDir was not specified when resolving the script tag in Vue SFC.

resolveDir: normalizePath(path.dirname(p)),

build.onLoad({ filter: /.*/, namespace: 'script' }, ({ path }) => {
return scripts[path]
})

So I added them.

No loader is configured for ".vue" files: src/components/ImportType.vue

But then this new error happened. This was happening because onResolve is not called for glob imports (evanw/esbuild#3317).
https:/vitejs/vite-plugin-vue/blob/e8503055b0ed29aa102b1f5b9a27f6b27a1f9713/playground/ssr-vue/src/pages/Home.vue#L31-L33
So I added a onLoad hook for namespace: 'file'.

// extract scripts inside HTML-like files and treat it as a js module
build.onLoad(
{ filter: htmlTypesRE, namespace: 'html' },
htmlTypeOnLoadCallback,
)
// the onResolve above will use namespace=html but esbuild doesn't
// call onResolve for glob imports and those will use namespace=file
// https:/evanw/esbuild/issues/3317
build.onLoad(
{ filter: htmlTypesRE, namespace: 'file' },
htmlTypeOnLoadCallback,
)

(TODO: check if this solves vitejs/vite-plugin-vue#253)

Error: The following dependencies are imported but could not be resolved:
@vitejs/test-dep-import-type/deep (imported by C:/Users/green/Documents/GitHub/vite-plugin-vue/playground/ssr-vue/src/components/ImportType.vue?id=0)
Are they installed?

And but then, this new error happend. This was happening because extractImportPaths was appending import '@vitejs/test-dep-import-type/deep' to the content of script tag. (related PR: #5850)

/**
* when using TS + (Vue + `<script setup>`) or Svelte, imports may seem
* unused to esbuild and dropped in the build output, which prevents
* esbuild from crawling further.
* the solution is to add `import 'x'` for every source to force
* esbuild to keep crawling due to potential side effects.
*/
function extractImportPaths(code: string) {
// empty singleline & multiline comments to avoid matching comments
code = code
.replace(multilineCommentsRE, '/* */')
.replace(singlelineCommentsRE, '')
let js = ''
let m
importsRE.lastIndex = 0
while ((m = importsRE.exec(code)) != null) {
js += `\nimport ${m[1]}`
}
return js
}

// append imports in TS to prevent esbuild from removing them
// since they may be used in the template
const contents =
content +
(loader.startsWith('ts') ? extractImportPaths(content) : '')

@vitejs/test-dep-import-type/deep is a type only file and Vite fails to resolve that.
I guess this is the same error with vitejs/vite-plugin-vue#24.
We can change the test in plugin-vue to use import type for now as it wasn't working from before.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red requested a review from bluwy October 4, 2023 12:26
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Make sense to me 👍 Thanks for looking into this

@bluwy bluwy merged commit 3615c68 into vitejs:main Oct 9, 2023
10 checks passed
@sapphi-red sapphi-red deleted the fix/esbuild-glob-resolve branch October 10, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants