Skip to content

Add TextEncoder.prototype.encodeInto #26904

Closed
Closed
Add TextEncoder.prototype.encodeInto#26904
Assignees
JungMinu
Labels
bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.encodingIssues and PRs related to the TextEncoder and TextDecoder APIs.feature requestIssues requesting new Node.js features.

Description

@RReverser
RReverser
opened on Mar 25, 2019

Is your feature request related to a problem? Please describe.

TextEncoder.prototype.encodeInto is a new method available in latest Chrome and Firefox that allows efficient encoding of strings into a given buffer, avoiding intermediate allocation and copying.

Its primary usecase is a more efficient way to pass strings to WASM (one of the most expensive things one has to do when communicating with JavaScript) and it's already leveraged by Rust wasm-bindgen where available. wasm-bindgen has support for a Node.js target, so it would be great to speed things up here too.

Describe the solution you'd like

Adding native encodeInto method to the TextEncoder class already available in util module.

Please describe the desired behavior.

MDN: https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto

Describe alternatives you've considered

Sending PR to wasm-bindgen to wrap memory into a Node-specific Buffer and use Buffer.prototype.write instead.

It has very similar semantics, and produces similar speed-ups (up to 35% on some of my string-interaction-heavy WASM libraries), but I wanted to see if it's possible to solve this on the Node.js side first so that the same method could be used for all platforms.

cc @jasnell as the author of the original implementation of WHATWG Encoding API in Node.js (#13644)

Activity

  1. RReverser
    added
    bufferIssues and PRs related to the buffer subsystem.
    on Mar 25, 2019
  2. devsnek
    added
    encodingIssues and PRs related to the TextEncoder and TextDecoder APIs.
    feature requestIssues requesting new Node.js features.
    on Mar 25, 2019

  3. jasnell commented on Mar 25, 2019

    @jasnell
    jasnell
    on Mar 25, 2019
    Member

    Should be fairly straightforward to implement. Interested in giving it a try?

  4. RReverser commented on Mar 25, 2019

    @RReverser
    RReverser
    on Mar 25, 2019
    MemberAuthor

    Potentially, but currently pretty busy with another higher-level project (in which I found out this perf differentiator between running my WASM in different environments).

    If nobody takes this before, can give it a try sometime later.

    Do you think it would be reasonable if I put good first issue label on this?

  5. jasnell commented on Mar 25, 2019

    @jasnell
    jasnell
    on Mar 25, 2019
    Member

    Labeling it good first issue should be fine but it will require a bit deeper knowledge than a beginner might have, and would likely require a bit of C++

  6. RReverser commented on Mar 25, 2019

    @RReverser
    RReverser
    on Mar 25, 2019
    MemberAuthor

    Yeah, depends whether that targets beginners in general or just beginners to the project... I think I can put "good first issue" + "C++" to make it more clear.

  7. RReverser
    added
    c++Issues and PRs that require attention from people who are familiar with C++.
    good first issueIssues that are suitable for first-time contributors.
    on Mar 25, 2019

  8. RReverser commented on Mar 25, 2019

    @RReverser
    RReverser
    on Mar 25, 2019
    MemberAuthor

    One difference with Buffer::write I should mention is that encodeInto additionally returns number of string code units written, not just bytes. This is important to allow partial writes. (In theory people using Buffer::write for partial writes would need something like that too, but somehow they got away without it?)

  9. RReverser
    added a commit that references this issue on Mar 25, 2019

    Add specialised passStringToWasm for Node.js

    12011f4
  10. JungMinu
    self-assigned this
    on Mar 25, 2019

  11. JungMinu commented on Mar 25, 2019

    @JungMinu
    JungMinu
    on Mar 25, 2019
    Member

    going to take care of this

  12. zandaqo commented on Jul 5, 2019

    @zandaqo
    zandaqo
    on Jul 5, 2019

    This would be really helpful for anyone dealing with binary. As much as I like Buffer, it's not an option for isomorphic code. Is there any ETA on this one yet?

  13. AtticusYang commented on Jul 23, 2019

    @AtticusYang
    AtticusYang
    on Jul 23, 2019

    @RReverser ,I tried to implement this task and I did it. but I am confused by the test case in
    https://github.com/nodejs/node/blob/master/test/fixtures/wpt/encoding/encodeInto.any.js,
    when input string is "input": "\u{1D306}", the expected read is 2, but the actual is 4, i also did a simple test as followed, the byte length of "\u{1D306}" is 4.

    let a = "\u{1D306}";
console.log(a);
console.log(Buffer.byteLength(a, 'utf8'));

    and the dest is a uint8array, the byteOffset is 0 and the length is 4.

    so I wonder the test case is right or wrong, thanks!

  14. 5 remaining items

  15. AtticusYang
    added a commit that references this issue on Sep 4, 2019

    src: add encodeInto to TextEncoder

    91d61fb
  16. addaleax
    removed
    good first issueIssues that are suitable for first-time contributors.
    on Sep 10, 2019
  17. addaleax
    added a commit that references this issue on Sep 10, 2019

    util: add encodeInto to TextEncoder

    01698e6
  18. addaleax
    closed this as completedin 7c9ee6don Sep 13, 2019
  19. targos
    added a commit that references this issue on Sep 20, 2019

    util: add encodeInto to TextEncoder

    11d1ef9

  20. zandaqo commented on Sep 23, 2019

    @zandaqo
    zandaqo
    on Sep 23, 2019

    I was playing with the latest nightly build (Sep 23) and noticed an odd behavior when non-string is used as a source:

    const encoder = new TextEncoder();
const destination = new Uint8Array(10);

encoder.encodeInto(null, destination);
//=> TypeError: The "src" argument must be of type string. Received type object

    While the spec does say that the source should be a USVString, its current implementations in both Chrome and Firefox opt for converting the source into a string first instead of throwing an error. So, are we sticking with the current implementation, that is, Node.js going to differ from browsers and throw if a non-string source is provided?

    Also, this throwing makes less sense considering that we are ok with non-string values in TextEncoder#encode:

    const encoder = new TextEncoder();
encoder.encode(null)
//=> Uint8Array [ 110, 117, 108, 108 ]
  21. BridgeAR
    added a commit that references this issue on Sep 25, 2019

    util: add encodeInto to TextEncoder

    772a5e0
  22. sjarmak
    added a commit that references this issue on Jul 27, 2026

    util: add encodeInto to TextEncoder

    7c9ee6d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.encodingIssues and PRs related to the TextEncoder and TextDecoder APIs.feature requestIssues requesting new Node.js features.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions