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

URL: changes to file URL path normalization #35429

Closed
annevk opened this issue Sep 30, 2020 · 7 comments
Closed

URL: changes to file URL path normalization #35429

annevk opened this issue Sep 30, 2020 · 7 comments
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@annevk
Copy link

annevk commented Sep 30, 2020

See whatwg/url#544 and web-platform-tests/wpt#25716 for new tests.

cc @jasnell

@bmeck
Copy link
Member

bmeck commented Sep 30, 2020

Will have to check if this messes with any existing stuff using the URL resolver

@watilde watilde added the whatwg-url Issues and PRs related to the WHATWG URL implementation. label Oct 4, 2020
@watilde
Copy link
Member

watilde commented Oct 4, 2020

I implemented the updated logic of URL at #35477 and the below tests failed atm as they seem to be depending on URL:

  • test/parallel/test-inspector-bindings.js
  • test/parallel/test-inspector-multisession-js.js
  • test/parallel/test-policy-dependencies.js
  • test/parallel/test-policy-integrity-flag.js
  • test/parallel/test-policy-scopes.js
  • test/parallel/test-inspector-connect-main-thread.js
  • test/parallel/test-worker-debug.js
  • test/sequential/test-inspector.js
  • test/sequential/test-inspector-break-when-eval.js
  • test/sequential/test-inspector-debug-brk-flag.js
  • test/sequential/test-inspector-exception.js
  • test/sequential/test-inspector-resource-name-to-url.js

Full log: https://gist.github.com/watilde/ff9051a491cdfe0100f10cf525f7bed5

I'm figuring out how we can make them pass without breaking change in the existed modules other than URL. Any advice would be appreciated.

@guybedford
Copy link
Contributor

@watilde if you can track down to what URL inputs and outputs those cases correspond to that would help a lot to determine if these changes should be considered breaking for the module system.

@watilde
Copy link
Member

watilde commented Oct 4, 2020

I just added a full log to my first comment. You can search by "Command:" to find failed cases.

@guybedford
Copy link
Contributor

@watilde thanks, I already looked through them actually but couldn't see which URL calls they corresponded to without running the build myself hence my comment.

@watilde
Copy link
Member

watilde commented Oct 5, 2020

@guybedford Thank you for your help! To share where the errors happen, I investigated codes and could figure out how to fix them. Now the PR affects only whatwg-url :)

@guybedford
Copy link
Contributor

Glad you have tracked it down and thanks for driving this important feedback which also affects Deno and other file-based module resolvers.

joesepi pushed a commit to joesepi/node that referenced this issue Jan 8, 2021
Refs: whatwg/url#544
Refs: web-platform-tests/wpt#25716

PR-URL: nodejs#35477
Fixes: nodejs#35429
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants