Skip to content

stringTools: ES2026 proposal-arraybuffer-base64 support#18063

Draft
kzhsw wants to merge 14 commits intoBabylonJS:masterfrom
kzhsw:patch-2
Draft

stringTools: ES2026 proposal-arraybuffer-base64 support#18063
kzhsw wants to merge 14 commits intoBabylonJS:masterfrom
kzhsw:patch-2

Conversation

@kzhsw
Copy link
Contributor

@kzhsw kzhsw commented Mar 10, 2026

This commit adds support of native base64 encoding/decoding api support, which is benchmarked to be faster then the js impl. It checks native support of the native api excluding polyfills by core-js and es-shims on module load, and fallback to the original js impl for backward compatibility.
See this forum post for discussion and benchmark result: https://forum.babylonjs.com/t/es2026-proposal-arraybuffer-base64-support/62715

This commit adds support of native base64 encoding/decoding api support, which is benchmarked to be faster then the js impl.
It checks native support of the native api excluding polyfills by core-js and es-shims on module load, and fallback to the original js impl for backward compatibility.
See this forum post for discussion and benchmark result: <https://forum.babylonjs.com/t/es2026-proposal-arraybuffer-base64-support/62715>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support in stringTools for the TC39 Uint8Array base64 proposal APIs (toBase64 / fromBase64) to use native implementations when available (and fall back to the existing JS implementation otherwise), aiming for better performance.

Changes:

  • Introduces runtime detection (HasNativeBase64) and switches encode/decode implementations based on detected support.
  • Adds native encode/decode paths using Uint8Array.prototype.toBase64 and Uint8Array.fromBase64.
  • Refactors the existing JS implementations into explicit fallback functions.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +46 to +73
declare global {
interface Uint8Array<TArrayBuffer extends ArrayBufferLike = ArrayBufferLike> {
/**
* Converts the `Uint8Array` to a base64-encoded string.
* @param options If provided, sets the alphabet and padding behavior used.
* @returns A base64-encoded string.
*/
toBase64(options?: { alphabet?: "base64" | "base64url" | undefined; omitPadding?: boolean | undefined }): string;
}

interface Uint8ArrayConstructor {
/**
* Creates a new `Uint8Array` from a base64-encoded string.
* @param string The base64-encoded string.
* @param options If provided, specifies the alphabet and handling of the last chunk.
* @returns A new `Uint8Array` instance.
* @throws {SyntaxError} If the input string contains characters outside the specified alphabet, or if the last
* chunk is inconsistent with the `lastChunkHandling` option.
*/
fromBase64(
string: string,
options?: {
alphabet?: "base64" | "base64url" | undefined;
lastChunkHandling?: "loose" | "strict" | "stop-before-partial" | undefined;
}
): Uint8Array<ArrayBuffer>;
}
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The declare global augmentation adds Uint8Array.prototype.toBase64 / Uint8Array.fromBase64 to the public typings of this package, even though Babylon.js does not polyfill these methods. That makes consumer code type-check when calling toBase64() directly, but it can still crash at runtime in engines that don’t implement the proposal. Prefer keeping these typings local to this module (e.g., local helper interfaces / any casts around the feature-detected calls) so the rest of the codebase and downstream users don’t see these methods as universally available.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commited here: kzhsw@b0d0104

Copy link
Member

Choose a reason for hiding this comment

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

I personally would have kept your original one but added ? after the functions, e.g. toBase64?( and fromBase64?(. This is declaring those functions may or may not be there, but if they are there, this is the shape of them, which seems like exactly what we want since not all browsers implement the functions. The cast works too though if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commmited here: kzhsw@e35d68f

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +89 to +96
// e must be TypeError
// chrome: Method Uint8Array.prototype.toBase64 called on incompatible receiver null
// firefox: toBase64 method called on incompatible null
// webkit: Uint8Array.prototype.toBase64 requires that |this| be a Uint8Array
// core-js: Argument is not an Uint8Array
// es-shims: `this` value must be a Uint8Array'
const message = (e as Error).message;
return message !== "Argument is not an Uint8Array" && message !== "`this` value must be a Uint8Array'";
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

HasNativeBase64() relies on matching Error.message strings to reject specific polyfills. Exception messages are not standardized and can vary by browser version/build and can be localized, which can lead to incorrect detection and potentially using a polyfill while thinking it’s native (or vice versa). Consider a more robust check (for example, using Function.prototype.toString to ensure the implementation is native, and/or checking e instanceof TypeError without depending on message text).

Suggested change
// e must be TypeError
// chrome: Method Uint8Array.prototype.toBase64 called on incompatible receiver null
// firefox: toBase64 method called on incompatible null
// webkit: Uint8Array.prototype.toBase64 requires that |this| be a Uint8Array
// core-js: Argument is not an Uint8Array
// es-shims: `this` value must be a Uint8Array'
const message = (e as Error).message;
return message !== "Argument is not an Uint8Array" && message !== "`this` value must be a Uint8Array'";
// e must be a TypeError for a spec-compliant native implementation
const isTypeError = e instanceof TypeError;
// Detect native implementations without relying on error message strings.
let isNative = false;
try {
const fnToString = Function.prototype.toString;
const toBase64Source = fnToString.call(Uint8Array.prototype.toBase64);
const fromBase64Source = fnToString.call(Uint8Array.fromBase64);
const nativeCodePattern = /\[native code\]/;
isNative = nativeCodePattern.test(toBase64Source) && nativeCodePattern.test(fromBase64Source);
} catch {
// If we cannot reliably inspect the functions, treat them as non-native.
isNative = false;
}
return isTypeError && isNative;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The toString check will now work, core-js is intentionally designed to make its polyfills as indistinguishable as possible from native implementations (it even overrides Function.prototype.toString to return [native code]). There is no official API to detect whether something was polyfilled by core-js.

Copy link
Member

Choose a reason for hiding this comment

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

is depending on strings returned by the browser a safe way of doing that? is there no deterministic, future-safe way for that? I actually agree with copilot here.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, seems like a real issue. I assume as it suggests these strings could be different for different browsers or different languages?

Copy link
Contributor Author

@kzhsw kzhsw Mar 11, 2026

Choose a reason for hiding this comment

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

is there no deterministic, future-safe way for that?

@RaananW Yes there is, create a transient, empty iframe, and check prototype in its contentWindow, but creating an iframe is a heavy operation, and contentWindow can not be accessed synchronously after creating the iframe, so it would not be possible to do the check as a side effect

Copy link
Member

Choose a reason for hiding this comment

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

I get this. I still prefer not to depend on pre-defined strings (and trusting the framework(s) to not change these strings).
Is it an issue if someone polyfilled with core.js? would it fail, or simply be slower than native?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RaananW Yes, the polyfill is expected to be slower than true native impl, so I do not want stringTools to be slower for polyfill users after this change.

Copy link
Member

Choose a reason for hiding this comment

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

i get that. but this is a documentation issue more than a runtime issue IMO. if you use core-js and poly-filled a feature that is available in all major browsers, expect it to run slower. I don't think we need to cater to those using core-js in this case.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking something similar to @RaananW. If a project is including core-js or es-shim, it seems they are explicitly choosing to use that polyfill, so should we just let them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@RaananW RaananW left a comment

Choose a reason for hiding this comment

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

Just putting it here - this feature is already available in most (well all...) used browsers:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/toBase64

Comment on lines +89 to +96
// e must be TypeError
// chrome: Method Uint8Array.prototype.toBase64 called on incompatible receiver null
// firefox: toBase64 method called on incompatible null
// webkit: Uint8Array.prototype.toBase64 requires that |this| be a Uint8Array
// core-js: Argument is not an Uint8Array
// es-shims: `this` value must be a Uint8Array'
const message = (e as Error).message;
return message !== "Argument is not an Uint8Array" && message !== "`this` value must be a Uint8Array'";
Copy link
Member

Choose a reason for hiding this comment

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

is depending on strings returned by the browser a safe way of doing that? is there no deterministic, future-safe way for that? I actually agree with copilot here.

@sebavan
Copy link
Member

sebavan commented Mar 10, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

Reviewer - this PR has made changes to the build configuration file.

This build will release a new package on npm

If that was unintentional please make sure to revert those changes or close this PR.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

Snapshot stored with reference name:
refs/pull/18063/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18063/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18063/merge/index.html#WGZLGJ#4600

Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves):

https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/18063/merge
https://gui.babylonjs.com/?snapshot=refs/pull/18063/merge
https://nme.babylonjs.com/?snapshot=refs/pull/18063/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge#BCU1XR#0

If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 10, 2026

@kzhsw
Copy link
Contributor Author

kzhsw commented Mar 11, 2026

Here is the benchmark playground targeting this PR:
https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge#BASE64#1

@RaananW
Copy link
Member

RaananW commented Mar 11, 2026

Here is the benchmark playground targeting this PR: https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge#BASE64#1

am i right to understand that for : 1MB we are faster already?

image

@sebavan
Copy link
Member

sebavan commented Mar 11, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 11, 2026

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 11, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 11, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 11, 2026

@kzhsw
Copy link
Contributor Author

kzhsw commented Mar 12, 2026

am i right to understand that for : 1MB we are faster already?

@RaananW Nope, this benchmark uses playground targeting this PR, which benchmarks base64 after this PR against pure native api.
To compare before/after PR, you'll need to compare the playground targeting last release against playground targeting this PR

@sebavan
Copy link
Member

sebavan commented Mar 13, 2026

/azp run

@RaananW
Copy link
Member

RaananW commented Mar 18, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@RaananW
Copy link
Member

RaananW commented Mar 18, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

Snapshot stored with reference name:
refs/pull/18063/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18063/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18063/merge/index.html#WGZLGJ#4600

Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves):

https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/18063/merge
https://gui.babylonjs.com/?snapshot=refs/pull/18063/merge
https://nme.babylonjs.com/?snapshot=refs/pull/18063/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/18063/merge#BCU1XR#0

If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 18, 2026

Comment on lines +139 to +145
if (HasNativeBase64()) {
ImplEncodeArrayBufferToBase64 = NativeEncodeArrayBufferToBase64;
ImplDecodeBase64ToBinary = NativeDecodeBase64ToBinary;
} else {
ImplEncodeArrayBufferToBase64 = JsEncodeArrayBufferToBase64;
ImplDecodeBase64ToBinary = JsDecodeBase64ToBinary;
}
Copy link
Member

Choose a reason for hiding this comment

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

This introduces module level side effects. I think if we instead had a module level Lazy (packages\dev\core\src\Misc\lazy.ts), it would defer this logic from running until the first time it is needed and therefore not be considered a side effect and be tree-shakable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok with this but there are 2 things to remind:

  1. Currently stringTools have no import, is it ok to introduce an import?
  2. This PR is performance-targeted, using Lazy would introduce one more branch (if (this._factory)), and one more indirect call (get value()) compared to curr impl, this does not shows a performance advantage over inlining the branch, for example:
export const EncodeArrayBufferToBase64 = (buffer: ArrayBuffer | ArrayBufferView): string => {
    const bytes = ArrayBuffer.isView(buffer) ? new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength) : new Uint8Array(buffer);

    return bytes.toBase64 ? bytes.toBase64() : JsEncodeArrayBufferToBase64(bytes);
};

This impl adds one more branch, but calls are no longer indirect.
It seems Lazy is designed for heavy initialization like module imports, but in this case the initialization is trivial, it's just a condition check, so for better tree-shaking compatibility branching might be just ok.

Copy link
Member

Choose a reason for hiding this comment

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

Either one seems fine to me! I would probably just branch in this situation, like you suggest above. I expect the cost of the branch is extremely small in comparison to the base64 encode/decode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryantrem Commited here: kzhsw@d3c28c7

@ryantrem
Copy link
Member

@kzhsw, I created a PR to fix the build issue with the global type declarations here: kzhsw#1

@RaananW can you take a look and see if this is the right way to handle this?

@RaananW
Copy link
Member

RaananW commented Mar 19, 2026

@kzhsw, I created a PR to fix the build issue with the global type declarations here: kzhsw#1

@RaananW can you take a look and see if this is the right way to handle this?

I believe putting it in the declaration directory would be better, as this is the way we have configured all of our build tools to work. I prefer avoiding reference types. Linting will also fail. Try putting it in the browser.d.ts in the LibDeclaration directory instead.

@ryantrem
Copy link
Member

@kzhsw, I created a PR to fix the build issue with the global type declarations here: kzhsw#1
@RaananW can you take a look and see if this is the right way to handle this?

I believe putting it in the declaration directory would be better, as this is the way we have configured all of our build tools to work. I prefer avoiding reference types. Linting will also fail. Try putting it in the browser.d.ts in the LibDeclaration directory instead.

Sounds good, my PR into this PR has been updated with these changes. Tested locally and it seems ok to me!

kzhsw added 3 commits March 20, 2026 08:47
Move global declarations to a d.ts file
This should improve tree-shaking compatibility of this file as requested here: <BabylonJS#18063 (comment)>
}

function JsDecodeBase64ToBinary(base64Data: string): ArrayBuffer {
const decodedString = atob(base64Data);
Copy link
Member

Choose a reason for hiding this comment

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

Why not still call DecodeBase64ToString here?

@ryantrem ryantrem marked this pull request as draft March 20, 2026 18:43
@ryantrem
Copy link
Member

We're very close to the 9.0 release now. Let's wait until after the release to merge this change.

@ryantrem ryantrem added this to the 10.0 milestone Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants