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 wasmJs support #3152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JavierSegoviaCordoba
Copy link
Collaborator

The code inside the Wasm source sets is a copy of the JS source sets. I don't know if it would be better to create an intermediate target (something like the web) or keep the duplication for the future.

Copy link
Member

@oldergod oldergod left a comment

Choose a reason for hiding this comment

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

Looks good to me. I need Jake's eyes on it as well.

@@ -41,11 +41,11 @@ jobs:

- name: Test Native and JS
run: |
./gradlew -Dkjs=true -Dknative=true -Pswift=false samples:native:build samples:js:build samples:multi-target:build --stacktrace --warning-mode all
./gradlew -Dkjs=true -Dkwasm=true -Dknative=true -Pswift=false samples:native:build samples:js:build samples:multi-target:build --stacktrace --warning-mode all
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
./gradlew -Dkjs=true -Dkwasm=true -Dknative=true -Pswift=false samples:native:build samples:js:build samples:multi-target:build --stacktrace --warning-mode all
./gradlew -Dkjs=true -Dkwasm=true -Dknative=true -Pswift=false samples:native:build samples:js:build samples:wasm:build samples:multi-target:build --stacktrace --warning-mode all


- name: Test
run: |
./gradlew -Dkjs=false -Dknative=false -Pswift=false build --stacktrace --warning-mode all -x samples:native:build -x samples:js:build -x samples:multi-target:build
./gradlew -Dkjs=false -Dkwasm=false -Dknative=false -Pswift=false build --stacktrace --warning-mode all -x samples:native:build -x samples:js:build -x samples:multi-target:build
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
./gradlew -Dkjs=false -Dkwasm=false -Dknative=false -Pswift=false build --stacktrace --warning-mode all -x samples:native:build -x samples:js:build -x samples:multi-target:build
./gradlew -Dkjs=false -Dkwasm=false -Dknative=false -Pswift=false build --stacktrace --warning-mode all -x samples:native:build -x samples:js:build -x samples:wasm:build -x samples:multi-target:build

@JakeWharton
Copy link
Collaborator

I definitely don't think we should be duplicating the majority of files, especially when we'll also want to be adding wasmWasi as a supported target. The use of the hierarchy DSL should make it relatively trivial to add intermediate source sets based around functionality. It seems like we should have axes like "grpc" and "noGrpc" as well as "jvm" and "nonJvm" since that seems to be the source of most of the behavior differences which require the expect/actuals in the first place.

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