Skip to content

fix: Improve injectQuery path handling (fix #2422)#2435

Merged
yyx990803 merged 1 commit intovitejs:mainfrom
GrygrFlzr:fix-inject-query
Mar 15, 2021
Merged

fix: Improve injectQuery path handling (fix #2422)#2435
yyx990803 merged 1 commit intovitejs:mainfrom
GrygrFlzr:fix-inject-query

Conversation

@GrygrFlzr
Copy link
Member

@GrygrFlzr GrygrFlzr commented Mar 8, 2021 •
edited
Loading

Fixes #2422.

This PR updates injectQuery to handle Windows paths in a saner manner.

  • As url.parse has been deprecated in Node, relative paths are now resolved using new URL(url, 'relative:///').
  • When a Windows drive letter is erroneously used as a protocol (e.g. c:), it instead resolves it using url.pathToFileURL and removes the first character (a forward slash) so that the following code in importAnalysis can properly handle it as an in-root URL:

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

Lines 182 to 192 in 76f3535

// normalize all imports into resolved URLs
// e.g. `import 'foo'` -> `import '/@fs/.../node_modules/foo/index.js`
if (resolved.id.startsWith(root + '/')) {
// in root: infer short absolute path from root
url = resolved.id.slice(root.length)
} else if (fs.existsSync(cleanUrl(resolved.id))) {
// exists but out of root: rewrite to absolute /@fs/ paths
url = path.posix.join(FS_PREFIX + resolved.id)
} else {
url = resolved.id
}

@GrygrFlzr GrygrFlzr changed the title Improve injectQuery path handling (fix #2422) fix: Improve injectQuery path handling (fix #2422) Mar 8, 2021
This was referenced Mar 10, 2021
Closed
Closed
@antfu antfu added the p4-important Violate documented behavior or significantly improves performance (priority) label Mar 13, 2021
@lamualfa
Copy link

lamualfa commented Mar 14, 2021

Sorry, Please merge this soon. I can't continue my development process because of this error: (sveltejs/kit#460)

image

@yousufiqbal
Copy link

yousufiqbal commented Mar 14, 2021

Please merge.. waiting so long to try sveltekit.. Thanks

@yyx990803 yyx990803 merged commit a5412f8 into vitejs:main Mar 15, 2021
@lamualfa
Copy link

lamualfa commented Mar 15, 2021

Yes !!!

image

@lamualfa lamualfa mentioned this pull request Mar 16, 2021
Closed
This was referenced Mar 18, 2021
Closed
Merged
@sapphi-red sapphi-red mentioned this pull request Aug 20, 2022
Merged
9 tasks
@sapphi-red sapphi-red mentioned this pull request Oct 21, 2022
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antfu antfu antfu approved these changes

Assignees

No one assigned

Labels

p4-important Violate documented behavior or significantly improves performance (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows: Import normalization can incorrectly resolve as both in and out of root

5 participants

@GrygrFlzr @lamualfa @yousufiqbal @antfu @yyx990803