url: add value argument to has and delete methods

[sankalp1999]

The change aims to add value argument to two methods of URLSearchParams class i.e the has method and the delete method. For has method, if value argument is provided, then use it to check for presence. For delete method, if value argument provided, use it to delete.

Fixes: #47883

[nodejs-github-bot]

Review requested:

• @nodejs/url

[anonrig]

Thank you for the contribution.

Can you run git node wpt url and update URL web-platform tests? You need to install node-core-utils in order to get access to the script.

Additionally, can you update the documentation url.md to reflect the changes?

[sankalp1999]

@anonrig
Thanks for the direction, will do these.

Yes, I will update the documentation accordingly.

[sankalp1999]

@anonrig need your help.

one of the WPT is not working.
file name is: idlharness.any.js. I think it's this one and it's calling an internal test

  "historical.any.js": {
    "fail": {
      "expected": [
        "searchParams on location object",
        "URL: no structured serialize/deserialize support",
        "URLSearchParams: no structured serialize/deserialize support"
      ]
    }
  },
  "idlharness.any.js": {
    "fail": {
      "unexpected": [
        "URLSearchParams interface: operation delete(USVString)",
        "URLSearchParams interface: operation has(USVString)"
      ]
    }
  },
  "javascript-urls.window.js": {
    "skip": "requires document.body reference"
  },
  "percent-encoding.window.js": {
    "skip": "TODO: port from .window.js"
  },
  "url-setters-a-area.window.js": {
    "skip": "already tested in url-setters.any.js"
  }
}

Ran 30/33 tests, 3 skipped, 28 passed, 1 expected failures, 1 unexpected failures, 0 unexpected passes
[redacted]/node/test/common/wpt.js:751
        throw new Error(
        ^

Error: Found 1 unexpected failures. Consider updating test/wpt/status/url.json for these files:
idlharness.any.js
    at process.<anonymous> ([redacted]/node/test/common/wpt.js:751:15)
    at process.emit (node:events:511:28)

Node.js v21.0.0-pre
Command: out/Release/node [redacted]node/wpt/test-url.js

[sankalp1999]

I think it's supposed to fail because the URLSearchParams interface changes haven't reached the main branch.

https://github.com/whatwg/url/pull/735/files

 "idlharness.any.js": {
    "fail": {
      "unexpected": [
        "URLSearchParams interface: operation delete(USVString)",
        "URLSearchParams interface: operation has(USVString)"
      ]
    }
  }
  

[sankalp1999]

@anonrig need your help with above.

[debadree25]

Hey the spec PR has landed, I am restarting the CI jobs should pass now i think

[debadree25]

Ah i think setting value = undefined is whats required since value is option per the spec

[sankalp1999]

Thanks @debadree25 I think that fixed it.
Have pushed the changes. Let's run CI. thanks.

[anonrig]

The tests are failing due to the WPT update, and unrelated to this pull request. I'll update the WPT in a different pull request and fix the issues, leaving you to only implement what this pull request was originally intended to include.

Edit: Actually it seems they are related. Only updating the WPT did not break any tests. @sankalp1999

[sankalp1999]

@anonrig @debadree25

I synced my branch, made a new node build. then ran sudo make test -j32

A couple of tests are failing. These are different from the ones that are failing in the PR (edit: I mean Github workflow). I will paste those messages in the next comment for visibility.

/Library/Developer/CommandLineTools/usr/bin/make -s jstest
ninja: Entering directory `out/Release'
ninja: no work to do.
=== release test-http-server-request-timeouts-mixed ===                       
Path: parallel/test-http-server-request-timeouts-mixed
node:assert:399
    throw err;
    ^

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert(request2.completed)

    at Timeout._onTimeout (/Users/sshubham/Desktop/open_source/node/test/parallel/test-http-server-request-timeouts-mixed.js:108:5)
    at listOnTimeout (node:internal/timers:573:17)
    at process.processTimers (node:internal/timers:514:7) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Node.js v21.0.0-pre
Command: out/Release/node [redacted]/node/test/parallel/test-http-server-request-timeouts-mixed.js


=== release test-net-connect-memleak ===                                      
Path: parallel/test-net-connect-memleak
node:events:489
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:217:20)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -54,
  code: 'ECONNRESET',
  syscall: 'read'
}

Node.js v21.0.0-pre
Command: out/Release/node --expose-gc [redacted]/node/test/parallel/test-net-connect-memleak.js


[06:22|% 100|+ 3859|-   2]: Done                                              

Failed tests:
out/Release/node [redacted]/node/test/parallel/test-http-server-request-timeouts-mixed.js
out/Release/node --expose-gc [redacted]/node/test/parallel/test-net-connect-memleak.js
make[1]: *** [jstest] Error 1
make: *** [test] Error 2

[nodejs-github-bot]

Landed in 9f3aacb

[debadree25]

Thank you for your contribution @sankalp1999 🎉🎉

[sankalp1999]

This is my first contribution to node. Looking forward to contribute more in the future.

I am grateful to @anonrig , @debadree25 and all the reviewers who helped me and reviewed my code.

I also improved my learning of the node repository and got an idea of how things work here.

Thanks a lot folks.

[anonrig]

This PR includes a change in benchmark/common.js and an update in web-platform-tests. It shouldn't have the do-not-land-v18 label.

[ruyadorno]

I'll try to land it in the current v18.18.0 proposal and run CI to see how it looks like, in case CI is green this should be good to go.