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 js browser target to paging extensions #3843

Merged
merged 10 commits into from
Jan 25, 2023
Merged

Conversation

sproctor
Copy link
Contributor

No description provided.

@hfhbd
Copy link
Collaborator

hfhbd commented Jan 23, 2023

Okay, why limit it to iOS and jvm (and now js) only? What about all other targets, eg darwin or native?

@veyndan
Copy link
Collaborator

veyndan commented Jan 23, 2023

Okay, why limit it to iOS and jvm (and now js) only? What about all other targets, eg darwin or native?

It's limited by what Multiplatform Paging supports. Currently it just supports iOS, JVM, and the (undocumented) JS targets. There's not a strong technical reason for not allowing paging-common to support any target. It's just that there (currently) aren't any UI components that support anything besides Android and iOS. This should change soon-ish with support for Jetbrains Compose UI as a target UI component.

@veyndan
Copy link
Collaborator

veyndan commented Jan 23, 2023

@sproctor Thanks for doing this! If you need help with the CI errors, please lmk

@sproctor
Copy link
Contributor Author

I tried my best on the CI errors, but I think they're now a bit past my expertise. Locally, I'm getting an internal compiler error when trying to run the tests and I'm also a bit confounded by the message generated here.

@hfhbd
Copy link
Collaborator

hfhbd commented Jan 24, 2023

I guess you can't make @before suspend. Workaround: use runTest and put your @before code there

@hfhbd
Copy link
Collaborator

hfhbd commented Jan 24, 2023

This won't work, you have to return runTest:

@Test fun foo() = runTest { }

And I mean this pattern:

private fun testing(action: suspend () -> Unit): TestResult { // or put the driver in the action
return runTest {
  before()
  action()
  after() // if needed
}

@Test fun foo() = testing {
}

@dellisd
Copy link
Collaborator

dellisd commented Jan 25, 2023

For testing you will also need to add some karma config files to copy the wasm file to a location that can be found by the test runner.

e.g. from coroutines-extensions
https://github.com/cashapp/sqldelight/blob/master/extensions/coroutines-extensions/karma.config.d/wasm.js

Copy link
Collaborator

@veyndan veyndan left a comment

Choose a reason for hiding this comment

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

Thanks!

@veyndan veyndan enabled auto-merge (squash) January 25, 2023 16:19
@veyndan veyndan merged commit a4ed59b into cashapp:master Jan 25, 2023
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.

None yet

4 participants