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: Improve injectQuery path handling (fix #2422) #2435

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

GrygrFlzr
Copy link
Member

@GrygrFlzr GrygrFlzr commented Mar 8, 2021

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:

// 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
@antfu antfu added the p4-important Violate documented behavior or significantly improves performance (priority) label Mar 13, 2021
@lamualfa
Copy link

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

image

@yousufiqbal
Copy link

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

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

Yes !!!

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
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