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

Update nodejs data for URLSearchParams #24998

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

skyclouds2001
Copy link
Contributor

@skyclouds2001 skyclouds2001 commented Nov 8, 2024

Summary

the support for URLSearchParams class is:

  • v7.0.0 - added as an internal instance (can be accessed via URL.prototype.searchParams)
    • constructor
    • append
    • delete
    • set
    • get
    • getAll
    • has
    • toString
    • @@iterator
  • v7.3.0 - exposed via url module, URL.prototype.searchParams no longer returns an empty list
    • keys
    • values
    • forEach
    • entries
  • v7.5.0 - extra support for constructor
    • constructor support a USVString
  • v7.7.0 - support sort() method
  • v7.10.0 - extra support for constructor
    • constructor support a record or sequence
  • v6.13.0 - backlogged url implement

Test results and supporting details

https://nodejs.org/docs/latest/api/globals.html#urlsearchparams
https://nodejs.org/docs/latest/api/url.html#class-urlsearchparams

nodejs/node#7448
https://nodejs.org/en/blog/release/v7.0.0/
https://nodejs.org/en/blog/release/v6.13.0/

nodejs/node#9484
https://nodejs.org/en/blog/release/v7.3.0/
https://nodejs.org/en/blog/release/v6.13.0/

nodejs/node#10399
nodejs/node#10801
nodejs/node#11057
nodejs/node#17365
https://nodejs.org/en/blog/release/v7.5.0/
https://nodejs.org/en/blog/release/v6.13.0/

nodejs/node#11098
https://nodejs.org/en/blog/release/v7.7.0/
https://nodejs.org/en/blog/release/v6.13.0/

nodejs/node#11060
https://nodejs.org/en/blog/release/v7.10.0/

nodejs/node#18281
https://nodejs.org/en/blog/release/v10.0.0/

nodejs/node#47885
https://nodejs.org/en/blog/release/v20.2.0/
https://nodejs.org/en/blog/release/v18.18.0/

image

image

image

image

Related issues

see also #24997
see also nodejs/node#55806

@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Nov 8, 2024
@skyclouds2001 skyclouds2001 marked this pull request as ready for review November 8, 2024 09:40
Comment on lines +35 to +40
{
"version_added": "7.0.0",
"version_removed": "7.5.0",
"partial_implementation": true,
"notes": "Only available as an internal instance."
},
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not exposed to the user, let's remove this entry.

Suggested change
{
"version_added": "7.0.0",
"version_removed": "7.5.0",
"partial_implementation": true,
"notes": "Only available as an internal instance."
},

Copy link
Contributor Author

@skyclouds2001 skyclouds2001 Nov 13, 2024

Choose a reason for hiding this comment

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

though the interface is not exposed directly, it can be accessed via URL.prototype.searchParams (i.e. new url.URL('https://github.com/mdn/browser-compat-data/pull/24998').searchParams.__proto__.constructor is the equal call to URLSearchParams)

and another reason is that there is a rule which force parent feature' version must not be greater or equal than child feature's version

see also #25001 (comment) and #25007 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if the note explains what the developer must do to access the instance instead, like Only accessible via `URL.searchParams`.. Could you rewrite the note accordingly (same for the other PRs)?

Copy link
Contributor

@caugner caugner Dec 20, 2024

Choose a reason for hiding this comment

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

I agree with Vinyl and made a similar suggestion in #25007 (comment).

Ultimately, this explicitness is useful for readers, because when you read "Only available as an internal instance.", the next question you ask yourself is: "How can I access this instance?"

api/URLSearchParams.json Outdated Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
api/URLSearchParams.json Show resolved Hide resolved
@caugner caugner added the nodejs Data about Node.js label Nov 12, 2024
@github-actions github-actions bot added the data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. label Nov 13, 2024
Co-authored-by: Claas Augner <495429+caugner@users.noreply.github.com>
Copy link

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Nov 18, 2024
@github-actions github-actions bot removed the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Nov 18, 2024
@github-actions github-actions bot added the size:l [PR only] 101-1000 LoC changed label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. nodejs Data about Node.js size:l [PR only] 101-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants