FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

url,lib: pass urlsearchparams-constructor.any.js by XadillaX · Pull Request #41197 · nodejs/node · GitHub

/ node Public

url,lib: pass urlsearchparams-constructor.any.js - #41197

Closed
XadillaX wants to merge 1 commit into
nodejs:masterfrom
XadillaX:urlsearchparams-constructor.any.js-2
ConversationCommits (1)ChecksFiles changed
Closed

url,lib: pass urlsearchparams-constructor.any.js#41197
XadillaX wants to merge 1 commit into
nodejs:masterfrom
XadillaX:urlsearchparams-constructor.any.js-2

Conversation

XadillaX commented Dec 16, 2021

Copy link
Copy Markdown
Contributor

NOTE: It's a reopening of #39944 because that PR was stuck.

According to WPT:

  1. URLSearchParams constructor should throw exactly TypeError if any
    Error occurrs.
  2. When a record passed to URLSearchParams constructor, two different
    key may result same after toUVString(). We should leave only the
    later one.

XadillaX requested a review from watilde December 16, 2021 08:58
nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Dec 16, 2021
XadillaX requested a review from addaleax December 17, 2021 05:11
jasnell reviewed Dec 17, 2021
View reviewed changes
Comment thread lib/internal/per_context/domexception.js
}

get code() { return 'ERR_INVALID_THIS'; }
function throwInvalidThisError(Base, type) {

jasnell Dec 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is the change to domexception.js related to the urlsearchparams change? If it is, it's not clear how. Also, it would likely be better to separate this out into a separate commit in the PR if it is related.

XadillaX Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This modification is to pass urlsearchparams-contructor.any.js too.

XadillaX Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

URLSearchParams constructor should throw exactly TypeError if any Error occurrs but not inheritance.

XadillaX Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://github.com/nodejs/node/blob/master/test/fixtures/wpt/url/urlsearchparams-constructor.any.js#L25-L26

aduh95 reviewed Dec 18, 2021
View reviewed changes
Comment thread lib/internal/url.js Outdated
XadillaX force-pushed the urlsearchparams-constructor.any.js-2 branch from c2e77f2 to c0a9720 Compare December 20, 2021 06:00
url,lib: pass urlsearchparams-constructor.any.js
59e2c32 
According to WPT:

1. `URLSearchParams` constructor should throw exactly `TypeError` if any
   Error occurrs.
2. When a record passed to `URLSearchParams` constructor, two different
   key may result same after `toUVString()`. We should leave only the
   later one.
XadillaX force-pushed the urlsearchparams-constructor.any.js-2 branch from c0a9720 to 59e2c32 Compare December 20, 2021 06:04

XadillaX commented Dec 23, 2021

Copy link
Copy Markdown
Contributor Author

/ping @aduh95

XadillaX commented Dec 27, 2021

Copy link
Copy Markdown
Contributor Author

@addaleax Hi Anna, would you please take a look at this PR?

XadillaX commented Dec 27, 2021

Copy link
Copy Markdown
Contributor Author

/cc @watilde

nodejs-github-bot commented Jan 4, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/41732/

This was referenced Jan 5, 2022
Open
Open
Open
Open
Open

nodejs-github-bot commented Jan 11, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/41814/

nodejs-github-bot commented Jan 11, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/41815/

nodejs-github-bot commented Jan 11, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/41816/

nodejs-github-bot commented Jan 11, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/41817/

XadillaX added a commit that referenced this pull request Jan 11, 2022
url,lib: pass urlsearchparams-constructor.any.js
38b7961 
According to WPT:

1. `URLSearchParams` constructor should throw exactly `TypeError` if any
   Error occurrs.
2. When a record passed to `URLSearchParams` constructor, two different
   key may result same after `toUVString()`. We should leave only the
   later one.

PR-URL: #41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>

XadillaX commented Jan 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Landed in 38b7961

XadillaX closed this Jan 11, 2022

aduh95 commented Jan 11, 2022

Copy link
Copy Markdown
Contributor

denoland/deno@b8303c7

Did you mean: "Landed in 38b7961"? 😅

github-actions Bot mentioned this pull request Jan 12, 2022
Open
20 tasks

XadillaX commented Jan 12, 2022

Copy link
Copy Markdown
Contributor Author

denoland/deno@b8303c7

Did you mean: "Landed in 38b7961"? 😅

Yes. I think I copied the wrong text🙈

github-actions Bot mentioned this pull request Jan 13, 2022
Open
18 tasks
github-actions Bot mentioned this pull request Jan 14, 2022
Open
22 tasks
targos pushed a commit that referenced this pull request Jan 14, 2022
url,lib: pass urlsearchparams-constructor.any.js
01ad8de 
According to WPT:

1. `URLSearchParams` constructor should throw exactly `TypeError` if any
   Error occurrs.
2. When a record passed to `URLSearchParams` constructor, two different
   key may result same after `toUVString()`. We should leave only the
   later one.

PR-URL: #41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
github-actions Bot mentioned this pull request Jan 15, 2022
Open
26 tasks
targos mentioned this pull request Jan 16, 2022
Merged
targos pushed a commit that referenced this pull request Jul 31, 2022
url,lib: pass urlsearchparams-constructor.any.js
69f9a3a 
According to WPT:

1. `URLSearchParams` constructor should throw exactly `TypeError` if any
   Error occurrs.
2. When a record passed to `URLSearchParams` constructor, two different
   key may result same after `toUVString()`. We should leave only the
   later one.

PR-URL: #41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
url,lib: pass urlsearchparams-constructor.any.js
bae9226 
According to WPT:

1. `URLSearchParams` constructor should throw exactly `TypeError` if any
   Error occurrs.
2. When a record passed to `URLSearchParams` constructor, two different
   key may result same after `toUVString()`. We should leave only the
   later one.

PR-URL: nodejs/node#41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell left review comments
aduh95 aduh95 left review comments
Lxxyx Lxxyx approved these changes
watilde Awaiting requested review from watilde
addaleax Awaiting requested review from addaleax

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL