-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 for hoisted scripts in project with spaces in the file path #5756
Conversation
🦋 Changeset detectedLatest commit: c6ddd87 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
const url = new URL('.' + filename, config.root); | ||
filename = prependForwardSlash(fileURLToPath(url)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work on Windows? I assume pathname
was being used here to ensure that we were getting the normalized /path/to/file
but fileURLToPath(url)
will give a platform-dependent file path. Maybe the result of that needs to be normalized before the forward slash is prepended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Windows tests pass, then I digress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there's something wrong here. We should be using fileURLToPath though, otherwise you get %20
for the spaces. Something else is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think prependForwardSlash(normalizePath(fileURLToPath(url)))
would be correct? Using normalizePath
from Vite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viteID did it. the problem was that fileURLToPath uses back slashes. viteID fixes it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Insert [space] pun here.
* Fix for hoisted scripts in project with spaces in the file path * Adding a changeset * Use viteID instead
Note that this will be backported to 1.0 after being cherry-picked.
Changes
Testing
Docs
N/A, bug fix