GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — stream: update TextEncoderStream to align with the latest spec by cola119 · Pull Request #44101 · nodejs/node

Conversation

@cola119

@cola119 cola119 commented Aug 2, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

  • transform(): https://encoding.spec.whatwg.org/#encode-and-enqueue-a-chunk
  • flush: https://encoding.spec.whatwg.org/#encode-and-flush

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.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 2, 2022
aduh95
aduh95 reviewed Aug 2, 2022
View reviewed changes
Comment thread lib/internal/webstreams/encoding.js Outdated
Comment on lines 6 to 7

@aduh95 aduh95 Aug 2, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@cola119 cola119 Aug 2, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done f2304b604d40fab394a3cbc81a04ed1d28dfa4b5

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
benjamingr
benjamingr approved these changes Aug 2, 2022
View reviewed changes
@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
stream: update TextEncoderStream to align the latest spec
ebbf25f
aduh95
aduh95 reviewed Aug 3, 2022
View reviewed changes
Comment thread lib/internal/webstreams/encoding.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@cola119
fixup! stream: update TextEncoderStream to align the latest spec
c44d983
@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 reviewed Aug 3, 2022
View reviewed changes
Comment thread lib/internal/webstreams/encoding.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@cola119
fixup! stream: update TextEncoderStream to align the latest spec
4a92843
aduh95
aduh95 approved these changes Aug 3, 2022
View reviewed changes
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Aug 3, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 3, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45818/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cola119 cola119 mentioned this pull request Aug 3, 2022
tools: add sort-requires rule to enforce {a, b, c} = require() is in alphabetical order #44116
Closed
@nodejs-github-bot

nodejs-github-bot commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45824/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Aug 4, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45832/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 4, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 4, 2022
@nodejs-github-bot
nodejs-github-bot merged commit c8bea71 into nodejs:main Aug 4, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 4, 2022

Copy link
Copy Markdown
Collaborator

Landed in c8bea71
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Aug 5, 2022
CI Reliability 2022-08-05 nodejs/reliability#337
Open
CI Reliability 2022-08-06 nodejs/reliability#338
Open
CI Reliability 2022-08-07 nodejs/reliability#339
Open
CI Reliability 2022-08-08 nodejs/reliability#340
Open
danielleadams pushed a commit that referenced this pull request Aug 16, 2022
@cola119 @danielleadams
stream: update TextEncoderStream to align the latest spec
e057480 
PR-URL: #44101
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@danielleadams danielleadams mentioned this pull request Aug 16, 2022
v18.8.0 release proposal #44246
Closed
ruyadorno pushed a commit that referenced this pull request Aug 23, 2022
@cola119 @ruyadorno
stream: update TextEncoderStream to align the latest spec
2f904bc 
PR-URL: #44101
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Aug 23, 2022
v18.8.0 release proposal #44353
Merged
@juanarbol

juanarbol commented Sep 29, 2022

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@juanarbol juanarbol added the backport-requested-v16.x label Sep 29, 2022
@cola119 cola119 mentioned this pull request Oct 30, 2022
lib: fix TypeError when converting a detached buffer source #44020
Merged
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingr benjamingr benjamingr approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@cola119 @nodejs-github-bot @juanarbol @benjamingr @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.