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

[v16.x backport] URL WPT changes #40383

Closed
wants to merge 11 commits into from
Closed

Conversation

targos
Copy link
Member

@targos targos commented Oct 9, 2021

Backport of #39848 and #39910.

foxxyz and others added 11 commits October 9, 2021 09:54
Main changes:

- Replace current HTML anchor generation to match
  header anchor generation in Github markdown.
- Remove unnecessary double namespacing on generated anchors/links (E.G.
  `esm.md#loaders` instead of `esm.md#esm_loaders`).
- Anchors/links are automatically prefixed with their respective modules
  when concatenated for usage in `all.html`.

Benefits:

- All anchor links within and between markdown API docs actually work.
- Adding new anchor links no longer requires contributors to generate
  the HTML docs first to look up the correct anchors.
- Anchors are much shorter.
- All previous anchor links are preserved by generating hidden legacy
  anchors.

PR-URL: nodejs#39304
Reviewed-By: Antoine du Hamel <[email protected]>
doc: update ESM hook examples

esm: fix unsafe primordial

doc: fix ESM example linting

esm: allow source of type ArrayBuffer

doc: update ESM hook changelog to include resolve format

esm: allow all ArrayBuffers and TypedArrays for load hook source

doc: tidy code & API docs

doc: convert ESM source table header from Title Case to Sentence case

doc: add detailed explanation for getPackageType

esm: add caveat that ESMLoader::import() must NOT be renamed

esm: tidy code declaration of getFormat protocolHandlers

doc: correct ESM doc link (bad conflict resolution)

doc: update ESM hook limitation for CJS

esm: tweak preload description

doc: update ESM getPackageType() example explanation

PR-URL: nodejs#37468
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: nodejs#40226
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#40300
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Refs: nodejs#36641
Refs: nodejs#36617 (comment)

Documentation-only deprecate `.aborted` property and `'abort'`,
`'aborted'` event in `http`, and suggest using the corresponding
Stream API instead.

Co-authored-by: Michaël Zasso <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Co-authored-by: Robert Nagy <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>

PR-URL: nodejs#36670
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs#36670

PR-URL: nodejs#40324
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Refs: nodejs#37468

PR-URL: nodejs#40275
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Derek Lewis <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
_subpath_ is not defined in this context. This is pretty clearly meant
to be _packageSubpath_, which is the second argument to
`PACKAGE_SELF_RESOLVE`

PR-URL: nodejs#40273
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
`resolve` does not return a `source` property

PR-URL: nodejs#40234
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: James M Snell <[email protected]>
`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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#39910
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v16.x whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Oct 9, 2021
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 9, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 9, 2021
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented Oct 9, 2021

@XadillaX

@targos
Copy link
Member Author

targos commented Oct 12, 2021

@nodejs/url

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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Nov 4, 2021
PR-URL: #39910
Backport-PR-URL: #40383
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
@targos
Copy link
Member Author

targos commented Nov 4, 2021

Thanks for the review! Landed in 0a8c331 and af4e682.

@targos targos closed this Nov 4, 2021
@targos targos deleted the url-v16 branch November 4, 2021 12:29
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.