Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(wasm): support inlined WASM in Node < v16 (fix #8620) #8622

Merged
merged 1 commit into from Jun 17, 2022
Merged

fix(wasm): support inlined WASM in Node < v16 (fix #8620) #8622

merged 1 commit into from Jun 17, 2022

Conversation

pastelmind
Copy link
Contributor

Node < v16 do not provide a global atob() function for decoding base64 strings, and must use Buffer.from() instead. Furthermore, Buffer.from(str, 'base64').toString() is much faster than atob() in Node. Thus, prefer to use Buffer.from() if possible.

Description

This fixes #8620 for Vite 3. The bug is caused because Vite generates code that uses atob(), which is available only on Node.js 16+. This PR solves this by:

  • Use Buffer.from() if it exists. This is faster than atob() (see below), so the code prioritizes it.
  • Check for existence of atob() before using it.
  • If neither options are available, throw an Error.

Note that Buffer.from() is much faster than atob():

// Node v16.15.1

> var bigStr = btoa('1234567890abcdefghijklmnopqrstuvwxyz'.repeat(100000));
> console.time('atob'); atob(bigStr); console.timeEnd('atob');
atob: 117.196ms
> console.time('Buffer.from'); Buffer.from(bigStr, 'base64').toString(); console.timeEnd('Buffer.from');
Buffer.from: 9.797ms

This is because atob() is intentionally unoptimized. (See rationale)

Additional context

vite-plugin-wasm has the same bug (see Menci/vite-plugin-wasm#3).


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bluwy bluwy added p3-minor-bug An edge case that only affects very specific usage (priority) feat: wasm labels Jun 16, 2022
Node < v16 do not provide a global `atob()` function for decoding base64
strings, and must use `Buffer.from()` instead. Furthermore,
`Buffer.from(str, 'base64').toString()` is much faster than `atob()` in
Node. Thus, prefer to use `Buffer.from()` if possible.
@pastelmind
Copy link
Contributor Author

This could be trivially backported to v2. Should I submit a separate PR to the v2 branch?

@bluwy
Copy link
Member

bluwy commented Jun 17, 2022

This could be trivially backported to v2. Should I submit a separate PR to the v2 branch?

Think that should be fine once this is merged.

@patak-dev patak-dev merged commit f586b14 into vitejs:main Jun 17, 2022
@patak-dev
Copy link
Member

@pastelmind PR welcome to cherry-pick this into the v2 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: wasm p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Library bundle with inlined WASM fails in Node < 16
3 participants