CircuitX - ImpressionEffect

[stagg]

Changes

• Adding some specific side-effects for use with clogging/analytics, typically for use in circuit presenters
• ImpressionEffect: A side effect that will run an impression once based on the retain strategy
• LaunchedImpressionEffect: A [LaunchedEffect] that will run a suspendable impression once, based on the retain strategy
• rememberImpressionNavigator: A [LaunchedImpressionEffect] that will also re-run the impression if it re-enters the composition after a navigation event.

[ZacSweers]

The tests are excellent! I do have a concern about them though - currently we don't run instrumentation tests on CI (we moved everything to robolectric). Is it possible to do that here? Relatedly, is it possible to test (a subset) of the impl in common tests for more multiplatform coverage?

Some other thoughts

• are we married to "sideffects" or should we just call it "effects"?
• I'm somewhat of the mind that we should actually make this a core circuit library, I think this pretty quickly becomes required for any app using Circuit
• Do you think we should account for LocalPinnedContainer (per our offline discussions wrt event listeners) here?

[ZacSweers]

chefskiss.png

[stagg]

The tests are excellent! I do have a concern about them though - currently we don't run instrumentation tests on CI (we moved everything to robolectric). Is it possible to do that here? Relatedly, is it possible to test (a subset) of the impl in common tests for more multiplatform coverage?

Turns out, we can! Interesting use of molecule and some expect/actual to get it all working though. Did that with c04d79f.

• are we married to "sideffects" or should we just call it "effects"?

Nope effects is better 👍🏻 162fd21

• I'm somewhat of the mind that we should actually make this a core circuit library, I think this pretty quickly becomes required for any app using Circuit

I think for the basic circuit use it's fine to not have it in foundation, and have it as an extension for advanced use. Don't really mind either way though 🤷🏻

• Do you think we should account for LocalPinnedContainer (per our offline discussions wrt event listeners) here?

Uses of this should be very deliberate so I don't think we need to do any extra support for sub-circuit use with this.
That said, added a test for the impression in a lazy container with 895b735

[ZacSweers]

🚀

Approving but could you add small doc to circuitx docs for the site before you merge?