Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

stream: update TextEncoderStream to align with the latest spec #44101

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Aug 2, 2022

This PR updated TextEncoderStream implementation to align with the latest WHATWG spec.

Test cases of /test/fixtures/wpt/encoding/streams/encode-utf8.any.js (WPT for TextEncoderStream) are all passed now. (The previous implementation didn't support surrogate pairs.)

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 2, 2022
Comment on lines 6 to 7
Uint8Array,
String
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Uint8Array,
String
String,
Uint8Array,

Copy link
Member Author

Choose a reason for hiding this comment

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

Done f2304b6

@cola119 cola119 requested a review from aduh95 August 2, 2022 23:53
@cola119 cola119 force-pushed the update-encoder-spec branch from f2304b6 to 03412dc Compare August 3, 2022 00:31
@cola119 cola119 force-pushed the update-encoder-spec branch from 03412dc to c44d983 Compare August 3, 2022 09:15
@cola119 cola119 requested a review from aduh95 August 3, 2022 09:16
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 3, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 4, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 4, 2022
@nodejs-github-bot nodejs-github-bot merged commit c8bea71 into nodejs:main Aug 4, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in c8bea71

danielleadams pushed a commit that referenced this pull request Aug 16, 2022
PR-URL: #44101
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno pushed a commit that referenced this pull request Aug 23, 2022
PR-URL: #44101
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
PR-URL: nodejs#44101
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@juanarbol
Copy link
Member

This is causing "wpt" tests in v16.x to fail. I will add the backport layer to this.

@cola119 cola119 changed the title stream: update TextEncoderStream to align the latest spec stream: update TextEncoderStream to align with the latest spec Feb 19, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants