airbnb / mavericks

Mavericks: Android on Autopilot
https://airbnb.io/mavericks/
Apache License 2.0
5.85k stars 499 forks source link

Remove createUnsafe and don't auto-subscribe on background threads #525

Closed marukami closed 3 years ago

marukami commented 3 years ago

Turns out createUnsafe does not work… at least, it breaks more than it fixes 😅 I double-checked all the test and ran a few demo apps to check.

I think a safer fallback is if the ViewModel is been created off the main thread then you must manually trigger the initialize by reading the ViewModel. The Mockables seemed fine in the limited test. helloDagger and the sample projects all seem to work fine with this method.

Sorry about this @gpeal I should have been more thorough in my testing, I got too focused on just the mockable not crashing.

gpeal commented 3 years ago

We could post a runnable to invoke this on the next frame, then check to make sure it's not already destroyed to initialize it.

Does this solution work for robolectric tests? Doesn't it essentially re-add the main thread check that broke robolectric in the first place?

gpeal commented 3 years ago

@elihart

marukami commented 3 years ago

Does this solution work for robolectric tests?

Yes, they all seem to pass, the last solution fails the tests.

Doesn't it essentially re-add the main thread check that broke robolectric in the first place?

robolectric works fine and does not throw as far as I can tell.

We could post a runnable to invoke this on the next frame, then check to make sure it's not already destroyed to initialize it.

Sure, let me update it this morning (my time)