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

[8.x] backport of util refactor and whatwg encoding #14585

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 1, 2017

Backport of #13803 and #13644 to v8.x-staging.

#13803 is required for #13644 to land cleanly.

This should go through a CI run before landing

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

* refactor util exports
* early capture of prototype methods
* use template strings and args consistently

PR-URL: nodejs#13803
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Provide an (initially experimental) implementation of the WHATWG Encoding
Standard API (`TextDecoder` and `TextEncoder`). The is the same API
implemented on the browser side.

By default, with small-icu, only the UTF-8, UTF-16le and UTF-16be decoders
are supported. With full-icu enabled, every encoding other than iso-8859-16
is supported.

This provides a basic test, but does not include the full web platform
tests. Note: many of the web platform tests for this would fail by default
because we ship with small-icu by default.

A process warning will be emitted on first use to indicate that the
API is still experimental. No runtime flag is required to use the
feature.

Refs: https://encoding.spec.whatwg.org/
PR-URL: nodejs#13644
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Aug 1, 2017
@jasnell
Copy link
Member Author

jasnell commented Aug 1, 2017

@jasnell jasnell requested a review from addaleax August 1, 2017 23:43
@addaleax
Copy link
Member

addaleax commented Aug 1, 2017

Thank you!

The second commit message doesn’t seem quite right, it should start with buffer: I think? LGTM apart from that.

@addaleax
Copy link
Member

addaleax commented Aug 1, 2017

Oh, that already happened during landing in master … hm. Should we fix that up here? I think our tooling would be okay with that and use buffer without making any more trouble.

@jasnell
Copy link
Member Author

jasnell commented Aug 1, 2017

Nope, the WHATWG Encoding stuff landed within util so the util: prefix is right. We need to revisit if that's the right place long term tho.

@addaleax
Copy link
Member

addaleax commented Aug 1, 2017

Oh, okay. In that case it’s just the original PR title that’s confusing. 😄

@jasnell
Copy link
Member Author

jasnell commented Aug 1, 2017

heh... oh right! completely forgot to change that!

addaleax pushed a commit that referenced this pull request Aug 2, 2017
* refactor util exports
* early capture of prototype methods
* use template strings and args consistently

Backport-PR-URL: #14585
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13803
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Aug 2, 2017
Provide an (initially experimental) implementation of the WHATWG Encoding
Standard API (`TextDecoder` and `TextEncoder`). The is the same API
implemented on the browser side.

By default, with small-icu, only the UTF-8, UTF-16le and UTF-16be decoders
are supported. With full-icu enabled, every encoding other than iso-8859-16
is supported.

This provides a basic test, but does not include the full web platform
tests. Note: many of the web platform tests for this would fail by default
because we ship with small-icu by default.

A process warning will be emitted on first use to indicate that the
API is still experimental. No runtime flag is required to use the
feature.

Backport-PR-URL: #14585
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>

Refs: https://encoding.spec.whatwg.org/
PR-URL: #13644
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@addaleax
Copy link
Member

addaleax commented Aug 2, 2017

CI failures are known flakes or infra problems

Landed in f593960 & 9a3fc10

@addaleax addaleax closed this Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants