-
-
Notifications
You must be signed in to change notification settings - Fork 6.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
fix: splitFileAndPostfix works as cleanUrl #12572
Conversation
Run & review this pull request in StackBlitz Codeflow. |
Looks like new URL('https://bar.com#foo?hello').hash === '#foo?hello'
new URL('https://bar.com#foo?hello').search === '' I think the original Later on, this ended up helping with paths like |
The PR now makes |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
/ecosystem-ci run astro iles |
/ecosystem-ci run iles |
Marking this as a fix. I think we should do it, but I'm not sure if in 4.3. |
It looks like the Iles fail isn't related to this PR, it was failing in others too (cc @ElMassimo in case you have some time to check with the current Vite main). @bluwy if the Astro fail is because of flakiness, I think we could merge this PR in 4.3. |
I'm not sure if it's a fluke since it's a new error I haven't seen before. re-running again |
/ecosystem-ci run astro |
So I believe this is how the behaviour had changed:
The fs refactor PR might've caused a regression, so this PR seems to be on the right track, but with only one change for the second case. I think this PR should be fine then 👍 |
I think you are right, looks like the logic the fs refactor wasn't right. I now updated this PR to ignore
|
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.
The new handling sounds good to me too 👍 I have one tiny nit, but this is good after that.
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Description
cleanUrl
andsplitFileAndPostfix
where working in different ways.This PR makes
cleanUrl
make them both work in the same way, possibly fixing issues with dependencies having#
in their path. The newcleanUrl
also avoids the second regex if it isn't needed.What is the purpose of this pull request?