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

feat(wasm): option for initWasm() to return the instance (#3847 #5615) #7181

Closed
wants to merge 1 commit into from

Conversation

Menci
Copy link
Contributor

@Menci Menci commented Mar 5, 2022

Description

In Vite's builtin wasm plugin, the exported function initWasm returns the exports property of the WASM instance, which is not always what we want. Some needs the module instance itself (e.g. #3847 #5615).

I added an option getInstance as initWasm's 2-nd parameter. When pass true it will return the instance instead of exports. This is a non-breaking change since omitting the parameter we still get the old behavior.

This PR will close #3847 and close #5615.

Additional context


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.

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

This makes the syntax a bit awkward if we want to get the instance. A couple alternatives I can think of:

  1. Introduce ?instance suffix so the wasm function returns the instance
  2. Support a __vite_instance property to the first parameter of initWasm()

Otherwise if we do want to introduce a new parameter, making in an object form would be more future-proof.

I'll put this in the next team meeting.

@bluwy bluwy added the p2-to-be-discussed Enhancement under consideration (priority) label Mar 6, 2022
@Menci
Copy link
Contributor Author

Menci commented Mar 6, 2022

Maybe change to initWasm(imports, { instance: true })?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: wasm p2-to-be-discussed Enhancement under consideration (priority)
Projects
None yet
2 participants