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.

Sign up for GitHub

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

Jump to bottom

URL: URL.canParse() #39069

Merged
merged 2 commits into from Mar 20, 2023
Merged

URL: URL.canParse() #39069

merged 2 commits into from Mar 20, 2023

Conversation

annevk
Copy link
Member

@annevk annevk commented Mar 18, 2023

@annevk annevk requested a review from ricea March 18, 2023 08:16
@wpt-pr-bot wpt-pr-bot added the url label Mar 18, 2023
ricea
ricea approved these changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@ricea ricea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

url/url-statics-canparse.any.js Outdated
"expected": true
}
].forEach(({ url, base, expected }) => {
test(t => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: t is unused.

@annevk annevk merged commit 7c5c3cc into master Mar 20, 2023
19 checks passed
@annevk annevk deleted the annevk/canparse branch March 20, 2023 09:13
This was referenced Mar 20, 2023
Closed
Closed
Merged
annevk added a commit to whatwg/url that referenced this pull request Mar 20, 2023
@annevk
Add URL.canParse()
ae3c28b 
Tests: web-platform-tests/wpt#39069.

Fixes #713.
@CYBAI CYBAI mentioned this pull request Mar 22, 2023
Closed
@zloirock
Copy link

zloirock commented Apr 6, 2023

I think that also should be added tests for something like URL.canParse({ toString() { throw Error() } }).

@zloirock
Copy link

One more case.

@annevk
Copy link
Member Author

annevk commented Apr 11, 2023

Create a PR? Happy to review next week.

@zloirock zloirock mentioned this pull request Apr 11, 2023
Closed
@zloirock
Copy link

Similar Deno bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricea ricea ricea approved these changes

@domenic domenic Awaiting requested review from domenic

@GPHemsley GPHemsley Awaiting requested review from GPHemsley

@mikewest mikewest Awaiting requested review from mikewest

@TimothyGu TimothyGu Awaiting requested review from TimothyGu

Assignees

@domenic domenic

Labels
url
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@annevk @zloirock @ricea @domenic @wpt-pr-bot