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

Wasm fn feature gate #102

Merged
merged 2 commits into from
Feb 5, 2019
Merged

Wasm fn feature gate #102

merged 2 commits into from
Feb 5, 2019

Conversation

zrzka
Copy link
Contributor

@zrzka zrzka commented Feb 5, 2019

Problem

  • cdsl have a function generate_ui
  • if reconfix does use cdsl as a dependency, this generate_ui fn is in the reconfix bindings as well, which is something I do not want
  • if I define generate_ui in the reconfix, it doesn't compile, because linker complaints about two functions with the same name

In other words. If cdsl is used to generate balena-cdsl NPM package, it's good as it is now. But if you'd like to use cdsl as a dependency and you do not want to have cdsl JS bindings in your NPM package (like reconfix), there's no way.

What this PR does

  • adds feature disable-wasm-bindings
  • wasm.rs is gated with target arch wasm32 & not(feature = "disable-wasm-bindings"))

By default, nothing changes. But for the reconfix I can say something like - I'd like to use cdsl, but without cdsl JS bindings.

Feature name

Initially, I was thinking about a feature called npm-package and build scripts update. I have found that wasm-pack build does support -- --features "npm-package" (added to 0.6). Unfortunately, wasm-pack test doesn't. Thus it must be enabled by default and we have to introduce disabling feature. Then the disable-wasm-bindings it is.

Robert Vojta added 2 commits February 5, 2019 08:33
Signed-off-by: Robert Vojta <robert@balena.io>
Change-type: patch
Signed-off-by: Robert Vojta <robert@balena.io>
@zrzka zrzka requested a review from cyplo February 5, 2019 07:43
@zrzka
Copy link
Contributor Author

zrzka commented Feb 5, 2019

I'll add this to temen as well once we agree on a way how to do it.

@zrzka
Copy link
Contributor Author

zrzka commented Feb 5, 2019

Related issues I just created:

Copy link
Contributor

@cyplo cyplo left a comment

Choose a reason for hiding this comment

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

some good research here :)

@zrzka zrzka merged commit 71c905b into master Feb 5, 2019
@zrzka zrzka deleted the wasm-fn-feature-gate branch May 1, 2019 09:53
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.

2 participants