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,lib: pass urlsearchparams-constructor.any.js #41197

Closed

Conversation

XadillaX
Copy link
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 XadillaX requested a review from watilde December 16, 2021 08:58
@nodejs-github-bot 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 XadillaX requested a review from addaleax December 17, 2021 05:11
}

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

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lib/internal/url.js Outdated Show resolved Hide resolved
@XadillaX XadillaX force-pushed the urlsearchparams-constructor.any.js-2 branch from c2e77f2 to c0a9720 Compare December 20, 2021 06:00
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 XadillaX force-pushed the urlsearchparams-constructor.any.js-2 branch from c0a9720 to 59e2c32 Compare December 20, 2021 06:04
@XadillaX
Copy link
Contributor Author

/ping @aduh95

@XadillaX
Copy link
Contributor Author

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

@XadillaX
Copy link
Contributor Author

/cc @watilde

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

XadillaX added a commit that referenced this pull request Jan 11, 2022
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
Copy link
Contributor Author

XadillaX commented Jan 11, 2022

Landed in 38b7961

@XadillaX XadillaX closed this Jan 11, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jan 11, 2022

denoland/deno@b8303c7

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

@XadillaX
Copy link
Contributor Author

denoland/deno@b8303c7

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

Yes. I think I copied the wrong text🙈

targos pushed a commit that referenced this pull request Jan 14, 2022
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>
thedull pushed a commit to thedull/node that referenced this pull request Jan 18, 2022
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#41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
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#41197
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
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>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants