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

Add electron support via --omit-imports #1958

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

silvanshade
Copy link
Contributor

This PR adds support for using wasm-bindgen-cli for Electron apps through the addition of two flags: --omit-imports and --preloads. See #1928 for context (the "Alternatives" section at the end of the OP).

@alexcrichton I think this is a better approach since it doesn't require adding a new target. I've also cleaned up the implementation and included your feedback from the original PR. If you think this looks good, we could go with this PR and I'll close the old one.

I'm also open to renaming the flags or modifying the documentation if you have thoughts about that.

@silvanshade
Copy link
Contributor Author

Just realized I forgot to add the blurb about the switches being experimental. I'll do that tomorrow.

@silvanshade silvanshade force-pushed the electron-support branch 2 times, most recently from 50b8a12 to 6e2da73 Compare January 17, 2020 16:37
@silvanshade silvanshade changed the title Add electron support via --omit-imports and --preloads Add electron support via --omit-imports Jan 17, 2020
@silvanshade
Copy link
Contributor Author

After further reflection, I've decided to just drop the --preloads flag and not include Electron preloads generation in this PR. The rationale is the same as what I described in the "alternatives" section in the original post of #1928.

At this point I'm not sure yet how useful automatic preloads generation will be so given the concern about long term maintenance I guess it's better to be on the conservative side.

This PR is now pretty simple and just adds a flag --omit-imports to disable generation of the import header section. This is enough to allow electron apps to work with either the bundler or web backend after writing a preloads file by hand.

@alexcrichton
Copy link
Contributor

Oh sorry for the delay, thanks for this though!

Can you expand the documentation a bit on this? Right now the docs sort of just say what the option itself already says. For example I don't really know easily what "imports" are in the js context, is this just like the section that brings names into the local scope?

@silvanshade
Copy link
Contributor Author

@alexcrichton sorry for the delay on this. I've updated the description with more detail. Does that work?

@alexcrichton alexcrichton merged commit 673e9b7 into rustwasm:master Feb 12, 2020
@alexcrichton
Copy link
Contributor

Thanks!

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.

3 participants