buffer: implement buffer.atob and buffer.btoa #37529

jasnell · 2021-02-26T18:02:40Z

Web Platform atob and btoa impl.

Just an initial get it working impl, not performance optimized.

Signed-off-by: James M Snell jasnell@gmail.com
Fixes: #3462

nodejs-github-bot · 2021-02-26T21:12:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/36387/

addaleax

Kinda feeling "meh" on repeating the web's mistakes, but I can see the point for web compat.

doc/api/buffer.md

benjamingr · 2021-02-27T12:40:22Z

I also don't really see the point for this API in Node.js, especially since ours (.toString on buffer) is arguably better and this API does not actually improve web compatibility by much since it's not global (and global probably isn't worth the breakage)

jasnell · 2021-02-27T14:36:11Z

Yep. It's been a long standing open feature request, and is implemented in other platforms, it was a quick impl so I figured let's see what the reaction is. I'm fine with not having this either. I'll close this and close the feature request issue as a #wontfix.

jasnell · 2021-03-17T17:52:27Z

I'm reopening this as I've had three separate, independent conversations with different folks in the past two weeks about the lack of atob/btoa in core. Yes, it's not as good as what we already have, and no, I don't see it as being critical, but the implementation is simple and easily maintained and it does improve cross-platform interop.

ljharb · 2021-03-17T17:54:11Z

@jasnell why couldn’t they be globals? (I’m sure there’s discussion somewhere, but I’d love to read it linked from here)

jasnell · 2021-03-17T18:05:06Z

Globals are semver major. If this lands I'll open a separate commit adding that. But this way it allows us the option of backporting the basic impls to 15 and 14

ljharb · 2021-03-17T18:05:57Z

awesome, sounds like a great plan

addaleax

Just making sure this doesn’t land until it has a big, bold warning against using it (or deprecation notice, tbh – I’d personally say that introducing this as a deprecated feature from the start would be just fine, but I’m guessing not everybody’s on the same page there…)

jasnell · 2021-03-17T18:27:05Z

I'd be -1 for introducing as deprecated but I'm definitely adding a notice in the docs as suggested. I definitely say we need a new status indicator in the docs that Is-Not-Deprecated-But-Is-Also-Not-Recommended

nodejs-github-bot · 2021-03-17T18:28:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/36762/ 💚

ljharb · 2021-03-17T18:31:33Z

The JS spec is going to use the term "legacy" for "things that are icky that we wish we could get rid of", if that helps :-)

jasnell · 2021-03-17T18:33:01Z

That does help, actually. There are a few APIs in core that are currently deprecated but really fall under that legacy category instead. I may have to open a PR adding that in as an option

doc/api/buffer.md

Signed-off-by: James M Snell <jasnell@gmail.com>

targos · 2021-03-17T19:58:14Z

lib/buffer.js

+
+const lazyInvalidCharError = hideStackFrames((message, name) => {
+  if (DOMException === undefined)
+    DOMException = internalBinding('messaging').DOMException;


why do we always have to lazily load this constructor?

Yeah, it's gotten a bit out of hand really. I'm doing it for consistency. I recently landed a PR that updated the various AbortError instances to eliminate this in multiple places but there are still more. What would be good is a PR that moves DOMException into `internal/errors' and replaces these lazy loads for that.

benjamingr · 2021-03-18T07:51:26Z

I'm still -0 on this (since right now we're just adding the bad methods without improving compatibility since they're not global) but I see the merit if they're global in the next major.

mcollina

lgtm

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell · 2021-03-18T19:09:21Z

Landed in 08770bc and c6855eb

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@Linkgoron

Notable changes: * buffer: * implement btoa and atob (James M Snell) #37529 * deps: * upgrade npm to 7.7.6 (Ruy Adorno) #37968 * doc: * add legacy status to stability index (James M Snell) #37784 * add @Linkgoron to collaborators (Nitzan Uziely) #37817 * http: * add http.ClientRequest.getRawHeaderNames() (simov) #37660

@Linkgoron

PR-URL: #37977 Notable changes: * buffer: * implement btoa and atob (James M Snell) #37529 * deps: * upgrade npm to 7.7.6 (Ruy Adorno) #37968 * doc: * add legacy status to stability index (James M Snell) #37784 * add @Linkgoron to collaborators (Nitzan Uziely) #37817 * http: * add http.ClientRequest.getRawHeaderNames() (simov) #37660

@Linkgoron

PR-URL: #37977 Notable changes: * buffer: * implement btoa and atob (James M Snell) #37529 * deps: * upgrade npm to 7.7.6 (Ruy Adorno) #37968 * doc: * add legacy status to stability index (James M Snell) #37784 * add @Linkgoron to collaborators (Nitzan Uziely) #37817 * http: * add http.ClientRequest.getRawHeaderNames() (simov) #37660

@Linkgoron

PR-URL: #37977 Notable changes: * buffer: * implement btoa and atob (James M Snell) #37529 * deps: * upgrade npm to 7.7.6 (Ruy Adorno) #37968 * doc: * add legacy status to stability index (James M Snell) #37784 * add @Linkgoron to collaborators (Nitzan Uziely) #37817 * http: * add http.ClientRequest.getRawHeaderNames() (simov) #37660

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 26, 2021

nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Feb 26, 2021

This comment has been minimized.

Sign in to view

addaleax reviewed Feb 26, 2021

View reviewed changes

doc/api/buffer.md Show resolved Hide resolved

doc/api/buffer.md Outdated Show resolved Hide resolved

doc/api/buffer.md Outdated Show resolved Hide resolved

doc/api/buffer.md Show resolved Hide resolved

addaleax reviewed Feb 26, 2021

View reviewed changes

doc/api/buffer.md Show resolved Hide resolved

jasnell closed this Feb 27, 2021

jasnell reopened this Mar 17, 2021

addaleax requested changes Mar 17, 2021

View reviewed changes

jasnell force-pushed the atob-btoa branch from 802710c to 32027d8 Compare March 17, 2021 18:21

jasnell force-pushed the atob-btoa branch from 32027d8 to f5b363a Compare March 17, 2021 18:27

jasnell requested a review from addaleax March 17, 2021 18:29

addaleax reviewed Mar 17, 2021

View reviewed changes

doc/api/buffer.md Outdated Show resolved Hide resolved

jasnell added 2 commits March 17, 2021 12:46

buffer: implement btoa and atob

ad44013

Signed-off-by: James M Snell <jasnell@gmail.com>

test: app atob web platform tests

12b20e5

Signed-off-by: James M Snell <jasnell@gmail.com>

jasnell force-pushed the atob-btoa branch from f5b363a to 12b20e5 Compare March 17, 2021 19:46

addaleax approved these changes Mar 17, 2021

View reviewed changes

jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 17, 2021

targos reviewed Mar 17, 2021

View reviewed changes

jasnell mentioned this pull request Mar 17, 2021

buffer: expose btoa and atob as globals #37786

Closed

mcollina approved these changes Mar 18, 2021

View reviewed changes

jasnell added a commit that referenced this pull request Mar 18, 2021

buffer: implement btoa and atob

08770bc

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell added a commit that referenced this pull request Mar 18, 2021

test: app atob web platform tests

c6855eb

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell closed this Mar 18, 2021

ruyadorno pushed a commit that referenced this pull request Mar 20, 2021

buffer: implement btoa and atob

dc9cd43

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

ruyadorno mentioned this pull request Mar 30, 2021

v15.13.0 proposal #37977

Merged

targos pushed a commit that referenced this pull request May 1, 2021

buffer: implement btoa and atob

bf2d9f2

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos pushed a commit that referenced this pull request May 1, 2021

test: app atob web platform tests

5c60087

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #37529 Fixes: #3462 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

danielleadams mentioned this pull request May 3, 2021

v14.17.0 proposal #38507

Merged

tniessen mentioned this pull request May 14, 2023

doc: Don't think atob and btoa should be marked as legacy #40754

Closed

MichaelDeBoey mentioned this pull request Aug 26, 2023

feat(remix-node): use built-in atob & btoa remix-run/remix#7206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buffer: implement buffer.atob and buffer.btoa #37529

buffer: implement buffer.atob and buffer.btoa #37529

jasnell commented Feb 26, 2021

This comment has been minimized.

nodejs-github-bot commented Feb 26, 2021

addaleax left a comment

benjamingr commented Feb 27, 2021

jasnell commented Feb 27, 2021

jasnell commented Mar 17, 2021

ljharb commented Mar 17, 2021

jasnell commented Mar 17, 2021

ljharb commented Mar 17, 2021

addaleax left a comment •

edited

jasnell commented Mar 17, 2021

nodejs-github-bot commented Mar 17, 2021 •

edited by jasnell

ljharb commented Mar 17, 2021

jasnell commented Mar 17, 2021

targos Mar 17, 2021

jasnell Mar 17, 2021

benjamingr commented Mar 18, 2021

mcollina left a comment

jasnell commented Mar 18, 2021

buffer: implement buffer.atob and buffer.btoa #37529

buffer: implement buffer.atob and buffer.btoa #37529

Conversation

jasnell commented Feb 26, 2021

This comment has been minimized.

nodejs-github-bot commented Feb 26, 2021

addaleax left a comment

Choose a reason for hiding this comment

benjamingr commented Feb 27, 2021

jasnell commented Feb 27, 2021

jasnell commented Mar 17, 2021

ljharb commented Mar 17, 2021

jasnell commented Mar 17, 2021

ljharb commented Mar 17, 2021

addaleax left a comment • edited

Choose a reason for hiding this comment

jasnell commented Mar 17, 2021

nodejs-github-bot commented Mar 17, 2021 • edited by jasnell

ljharb commented Mar 17, 2021

jasnell commented Mar 17, 2021

targos Mar 17, 2021

Choose a reason for hiding this comment

jasnell Mar 17, 2021

Choose a reason for hiding this comment

benjamingr commented Mar 18, 2021

mcollina left a comment

Choose a reason for hiding this comment

jasnell commented Mar 18, 2021

addaleax left a comment •

edited

nodejs-github-bot commented Mar 17, 2021 •

edited by jasnell