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.

Sign up for GitHub

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

Jump to bottom

Support separate wasm file as well as inlined wasm file #164

Open
isaac-mason opened this issue Sep 26, 2023 · 3 comments
Open

Support separate wasm file as well as inlined wasm file #164

isaac-mason opened this issue Sep 26, 2023 · 3 comments
Assignees
@isaac-mason
Labels
enhancement New feature or request

Comments

@isaac-mason
Copy link
Owner

isaac-mason commented Sep 26, 2023 •
edited
Loading

The main advantages of a separate wasm file are reduced bundle size and faster initialisation.

The downside is there are extra considerations required for supporting bundler configuration or serving the wasm from somewhere.

Continuing to support the existing inlined wasm flavour is important for ease of use.

There are several potential approaches. To name some:

  • Have two versions of the recast-navigation npm package, one with an inlined wasm, one with a separate wasm file
    • rapier.js takes this approach
  • Have one recast-navigation npm package, support passing an instance of @recast-navigation/wasm (separate file, inlined wasm, asm.js, other) to init
    • if there was a default, would need to make sure it could be tree-shaken
@isaac-mason isaac-mason changed the title Support loading external wasm file Support separate wasm file as well as inlined wasm file Sep 26, 2023
@isaac-mason isaac-mason added the enhancement New feature or request label Sep 26, 2023
@isaac-mason
Copy link
Owner Author

isaac-mason commented Jan 17, 2024 •
edited
Loading

wip branch trying the second approach here: https://github.com/isaac-mason/recast-navigation-js/tree/feat/specify-recast-impl

Right now the the wasm flavor (separate wasm file, not inlined like wasm-compat) doesn't work on node.

The emscripten build parameter ENVIRONMENT needs to be updated to include 'node' so there is node friendly logic for reading the wasm. However, these node code paths cause issues with some bundlers (at least webpack 5) due to the unrecognised node builtins. These warnings/errors can be surpressed and the build output will still work, but this is likely to cause confusion. Some bundlers such as vite don't have the issue.

Some options:

  • (short term) Accept and document that the wasm flavor doesn't work on node, and that wasm-compat should be used
  • Have a web + node wasm flavor, document the webpack 5 issue as a known issue
  • Have separate web and node wasm builds.
    • This isn't ideal, we'd need to have 4 builds (wasm-node, wasm-web, wasm-compat-node, wasm-compat-web) , and it may not be straightforward/possible to configure a package.json so the correct one is used automatically

@isaac-mason isaac-mason mentioned this issue Apr 10, 2024
Open
6 tasks
@isaac-mason isaac-mason mentioned this issue May 1, 2024
Closed
@isaac-mason
Copy link
Owner Author

78cf5df

@isaac-mason
Copy link
Owner Author

Starting with the short term option mentioned above

(short term) Accept and document that the wasm flavor doesn't work on node, and that wasm-compat should be used

@github-actions github-actions bot mentioned this issue May 4, 2024
Merged
@isaac-mason isaac-mason self-assigned this May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@isaac-mason isaac-mason

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@isaac-mason