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

Fix compatibility with JavaScriptKit 0.7 #281

Merged
merged 5 commits into from
Sep 30, 2020
Merged

Conversation

MaxDesiatov
Copy link
Collaborator

@MaxDesiatov MaxDesiatov commented Sep 28, 2020

This PR requires carton 0.6.0 that you can install from Homebrew as usual.

To cleanly manage scheduler closures, new JSScheduler class is introduced that conforms to OpenCombine's Scheduler protocol. I think it will be moved to OpenCombineJS in the future.

Basic things seem to work, but I think there's a noticeable performance regression somewhere. When I switch between navigation views, it takes almost a second to load everything. I've been staring at profiling data for a bit, JSString which was introduced in JavaScriptKit 0.7 is there somewhere, but I'm not sure what exactly is the culprit yet.
Update: I no longer can get the performance regression reproduced. 🤷‍♂️

@MaxDesiatov MaxDesiatov added the dependencies Updates to the project dependencies label Sep 28, 2020
@MaxDesiatov MaxDesiatov marked this pull request as ready for review September 28, 2020 19:52
@MaxDesiatov

This comment has been minimized.

@MaxDesiatov
Copy link
Collaborator Author

MaxDesiatov commented Sep 28, 2020

I can no longer reproduce the performance regression. carton 0.6.0 is now available on Homebrew, so I'd appreciate if you try things on your side. Regardless, it's time for setting up performance testing on CI...

@MaxDesiatov
Copy link
Collaborator Author

The CI failure was caused by some issues with GitHub Actions, now ready for review 🙂

@MaxDesiatov MaxDesiatov merged commit ee0006a into main Sep 30, 2020
@MaxDesiatov MaxDesiatov deleted the javascriptkit-0.7 branch September 30, 2020 09:17
MaxDesiatov added a commit to swiftwasm/OpenCombineJS that referenced this pull request Sep 30, 2020
`JSScheduler` is copied from TokamakUI/Tokamak#281.

You can see the `JSPromise` publisher in action in combination with `fetch` by running `carton dev` in the root directory. I saw some issues with `JSValueDecoder`, which is why it's not used in this PR. I will investigate `JSValueDecoder` issues separately.

* Add `JSScheduler` and `JSPromise` publisher

* Clean up publisher signature in `main.swift`

* Capture `self` weakly in `PromisePublisher.init`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Updates to the project dependencies
Development

Successfully merging this pull request may close these issues.

2 participants