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 support for WASM #277

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Add support for WASM #277

merged 3 commits into from
Dec 6, 2023

Conversation

eygraber
Copy link
Contributor

@eygraber eygraber commented Jul 12, 2023

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this a shared source set for js and wasmJs. The IDE shows some errors, but it seems to be building fine.

@evant
Copy link
Owner

evant commented Nov 23, 2023

Blocked by Kotlin/kotlinx.coroutines#3849

fyi, since this only needed for tests, it would be acceptable to skip those tests for the wasm target while it's still experimental

@eygraber
Copy link
Contributor Author

I don't think it's possible to skip coroutine tests for wasm because atomicfu and coroutines artifacts are clashing. Once the new version of coroutines releases, the commit commenting out coroutines can be reverted and this should be able to be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this because I kept having to make sure that I wasn't committing it. I think best practice is to track it in git, but I'll remove if you don't want it.

Copy link
Owner

Choose a reason for hiding this comment

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

I think best practice is to track it in git

If that's the case, then yeah it's fine to keep, I wasn't sure what was best practice here

Copy link
Owner

Choose a reason for hiding this comment

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

@evant
Copy link
Owner

evant commented Nov 28, 2023

So, in terms of skipping for wasm my thought would be to move the tests that use coroutines to a source set that excludes wasm, and the wasm target would be the one that doesn't include the coroutine dependency. It shouldn't be bringing in the conflicting dependencies at the same time right? Is there some reason that wouldn't work?

@eygraber
Copy link
Contributor Author

Is there some reason that wouldn't work?

It should work, but it might be worth waiting for the coroutines release. I'll check in with the kotlinx team to see if they have any kind of timeline.

@evant
Copy link
Owner

evant commented Dec 1, 2023

@eygraber
Copy link
Contributor Author

eygraber commented Dec 1, 2023

Yup, going to try updating to that soon. Are you ok with releasing the RC?

@eygraber
Copy link
Contributor Author

eygraber commented Dec 1, 2023

Rebased onto #333

@eygraber
Copy link
Contributor Author

eygraber commented Dec 6, 2023

OK should be ready to go. How do you feel about merging with coroutine 1.8-RC?

@evant
Copy link
Owner

evant commented Dec 6, 2023

It's not something we are distributing with so it should be fine

@evant evant merged commit e3dca3c into evant:main Dec 6, 2023
2 checks passed
@eygraber eygraber deleted the wasm branch June 17, 2024 16:34
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