-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
use href to get the full path of the file #4044
Conversation
jit/lib/setupContext.js
Outdated
@@ -229,7 +229,7 @@ function trackModified(files) { | |||
for (let file of files) { | |||
if (!file) continue | |||
|
|||
let pathname = url.parse(file).pathname | |||
let pathname = url.parse(file).href |
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.
Hey! Unfortunately this isn't the correct fix — the href
property includes the query string and that's the part we are trying to drop here.
I think the correct fix is going to be something like:
let parsed = url.parse(file)
let pathname = parsed.href.replace(parsed.hash, '').replace(parsed.search, '')
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 will try that change and make the appropriate updates to the PR
meanwhile I would like to ask if this is intended to run outside of a node environment. under what circumstances would there ever be a hash/query params. it would really help to understand how I could have caught this use case earlier
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.
ok, now the failed tests make more sense
I'll get right on those changes
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 the only time it happens is in some build tools like Vite that proxy URLs to the file system and add query string information. For example in Vite it might make a request for a Vue file just to get the CSS portion using a path that's sort of like this if I remember correctly:
http://localhost:3000/@/src/App.vue?lang=css
That gets converted to a file name when passed in to PostCSS and the query string is preserved, so we need to chop it off.
Codecov Report
@@ Coverage Diff @@
## master #4044 +/- ##
=======================================
Coverage 70.11% 70.12%
=======================================
Files 219 219
Lines 5455 5456 +1
Branches 1029 1029
=======================================
+ Hits 3825 3826 +1
Misses 1468 1468
Partials 162 162
Continue to review full report at Codecov.
|
fixes #4043