Skip to content

Conversation

@XadillaX
Copy link
Contributor

@XadillaX XadillaX commented Aug 23, 2021

url.idl defines URL's constructor as:

constructor(USVString url, optional USVString base);

idlharness.any.js checks its length as 1. So we should remove
constructor's second argument and use arguments[1] in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index

@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 Aug 23, 2021
@targos
Copy link
Member

targos commented Aug 23, 2021

I'd prefer to use a default value: constructor(input, base = undefined) {

`url.idl` defines URL's constructor as:

```
constructor(USVString url, optional USVString base);
```

`idlharness.any.js` checks its length as `1`. So we should remove
constructor's second argument and use `arguments[1]` in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

XadillaX added a commit that referenced this pull request Aug 26, 2021
`url.idl` defines URL's constructor as:

```
constructor(USVString url, optional USVString base);
```

`idlharness.any.js` checks its length as `1`. So we should remove
constructor's second argument and use `arguments[1]` in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index

PR-URL: #39848
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@XadillaX
Copy link
Contributor Author

Landed in 48655e1

@targos
Copy link
Member

targos commented Sep 6, 2021

This depends on the semver-major #39176. Would you like to backport?

targos pushed a commit to targos/node that referenced this pull request Oct 9, 2021
`url.idl` defines URL's constructor as:

```
constructor(USVString url, optional USVString base);
```

`idlharness.any.js` checks its length as `1`. So we should remove
constructor's second argument and use `arguments[1]` in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index

PR-URL: nodejs#39848
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Nov 4, 2021
`url.idl` defines URL's constructor as:

```
constructor(USVString url, optional USVString base);
```

`idlharness.any.js` checks its length as `1`. So we should remove
constructor's second argument and use `arguments[1]` in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index

PR-URL: #39848
Backport-PR-URL: #40383
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Nov 26, 2021
1 task
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