feat(remix-node): use built-in atob & btoa

[MichaelDeBoey]

• https://nodejs.org/dist/latest-v20.x/docs/api/globals.html#atobdata
• https://nodejs.org/dist/latest-v20.x/docs/api/globals.html#btoadata

[changeset-bot]

🦋 Changeset detected

Latest commit: ecfdd23

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@remix-run/node	Patch
@remix-run/architect	Patch
@remix-run/express	Patch
@remix-run/serve	Patch
@remix-run/testing	Patch
@remix-run/dev	Patch
create-remix	Patch
remix	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/css-bundle	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/react	Patch
@remix-run/server-runtime	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[shamsup]

I think this will resurface bugs like #1282

From linked node docs (followed link to buffer.atob alias)

This function is only provided for compatibility with legacy web platform APIs and should never be used in new code, because they use strings to represent binary data and predate the introduction of typed arrays in JavaScript. For code running using Node.js APIs, converting between base64-encoded strings and binary data should be performed using Buffer.from(str, 'base64') and buf.toString('base64')

[shamsup]

I see now that the parse/serialize is still using the escape and unescape step on the string beforehand, so it is functionally equivalent.

This does make me wonder - does v2 release offer a path to move off of atob and btoa entirely with safer methods? It would be a breaking change because cookies set from earlier remix versions will not decode correctly in v2 if it changes.

[MichaelDeBoey]

@shamsup atob & btoa are part of the standardized Web API

Looking at the Bun, Cloudflare & Deno docs, they have them already available, hence why we currently only polyfilled them in our Node runtime

• https://bun.sh/docs/runtime/web-apis
• https://developers.cloudflare.com/workers/runtime-apis/web-standards/#base64-utility-methods
• https://deno.land/[email protected]?s=atob
• https://deno.land/[email protected]?s=btoa

Even though these functions are marked as Legacy and the Node docs point towards using the Buffer class instead, the Buffer class is only partially available on Bun (see https://bun.sh/docs/runtime/nodejs-apis#buffer), only available on Cloudflare if you enable the nodejs_compat flag (see https://developers.cloudflare.com/workers/runtime-apis/nodejs/buffer) and not at all in Deno

Since the Node team is working on Web API compliancy and looking at nodejs/node#37529 & nodejs/node#37786, the Node team admitted that they added atob & btoa purely for Web API compliancy (but are in favor of using the Buffer class, hence them being marked as Legacy), so I'm quite certain they will keep atob & btoa in the long run
Maybe they will at some point convince Cloudflare/Deno (and other members of WinterCG) to implement Buffer natively as well, but that won't be soon I think 🤷‍♂️

If we look a bit deeper into nodejs/node#37529 & nodejs/node#37786, you can also see that Buffer is used as the implementation of atob & btoa by the Node team

So for now, I just wanted to remove the polyfills and use the global available functions instead.

[shamsup]

@MichaelDeBoey I understand the reasoning for this PR, thank you. I understand the atob/btoa API will likely never be removed from any platform for compatibility reasons, I was just wondering if it would be a good time to replace these and the workarounds implemented for utf-8 compatibility with more modern alternatives. I can open an RFC for this when I get time.
My initial reason for commenting was my fear of #1282 returning, but that won't affect this PR. 🚀

[MichaelDeBoey]

@shamsup If you think you have a better solution that's cross-platform, we would be happy to accept a PR for that

[brophdawg11]

I think we're doing the correct thing here now by not using the built ins, which according to the docs:

should never be used in new code

We're providing our own polyfills using the approach recommended by node, and only doing so in the node adapter.

[MichaelDeBoey]

@brophdawg11 I would disagree with using our own polyfills for something that all platform(s) support built-in now

As explained above, the Node team indeed prefers us to use the Buffer class, but as I also mentioned (and you can see in nodejs/node#37529 & nodejs/node#37786,) the global atob & btoa function already use the Buffer class under the hood

Having a polyfill for something that's cross-platform supported also means we'll have to maintain more code from our own for no reason at all in this case (the global functions are marked as Legacy, not Deprecated or Experimental)

[brophdawg11]

I know that node uses Buffer under the hood, and that's why I'm comfortable with our polyfill approach as it's doing exactly what they're doing - and using only non-Legacy APIs.

I may be a stickler here, and I'm happy to be overridden by @jacob-ebey / @mjackson who are far more familiar with the cross-platform nuances than I am - but I'm just not comfortable shipping code that relies on something that "should never be used in new code" 🤷

Don't users have the same choice here of not calling installGlobals if they prefer the native implementation?

[MichaelDeBoey]

@brophdawg11 As pointed out in my message before and as pointed out by the Node team in the linked issues/PRs, marking something as Legacy means it's a preference (at most a strong suggestion)
That together with the fact that the globals are already using their suggestion under the hood and that one of the main ideas behind Remix is "use the platform" and we want to have all platforms doing this as much as possible (so the goal is to remove all polyfills asap)

Those are all reasons I would say that using built-in atob & btoa is the way to go

it's doing exactly what they're doing

Another reason: what if the Node team has a better implementation at some point in time?
Then we'll have to update our polyfill, while we don't have that extra effort if we just keep using built-in globals

Don't users have the same choice here of not calling installGlobals if they prefer the native implementation?

That would mean they still have to polyfill all other things we polyfill themselves, just for using these 2 globals

[brophdawg11]

I thought about this some more and am coming around to it. With all the fetch polyfill discussion happening I was thinking about the fetch implications too much when considering atob/btoa. However, they are quite different considerations - the reasoning for which only occurred to me last night.

It boils down to:

1. Will most apps need this thing?
2. Is there a built-in + stable alternative?

fetch is something every app will need and there is no built-in alternative. With the native one being unstable, we have no choice but to offer them a "stable" one.

atob/btoa are different - they're not something most apps will need, but more importantly - there is a built-in + stable alternative in Buffer. So we don't have to offer them a "stable/reliable/non-legacy one". If an app decide's to use atob/btoa and get the native "legacy" one - that's the app choice to use a legacy API. If they don't want legacy they can use Buffer.

So two different scenarios but I think my brain was looking at them in the same light and looking at atob/btoa the wrong way.

[brophdawg11]

Let's rebase this onto release-next so we can merge it alongside #7293

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.6 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-9288516-20230902 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.7 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.7 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-481f73e-20230906 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.8 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-1a57073-20230907 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.9 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-1fac238-20230908 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.10 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-3646f91-20230914 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!