I'm trying to associate some data with my componen...

# decompose

[Sean Proctor]

I'm trying to associate some data with my components that is intended to survive configuration changes, so I've stored it in instanceKeeper . When the component leaves the stack, onDestroyed is called on the data. When the component is re-added to the stack, the same data is returned by instanceKeeper.getOrCreate that had been previously destroyed. This is with 3.0.0-alpha05.

[Arkadii Ivanov]

Do you mean Instance#onDestroy is called? Do you mean the same instance (by reference) is returned later again?

[Sean Proctor]

Yes, both, exactly.

[Arkadii Ivanov]

Do you have a reproducer for this?

[Arkadii Ivanov]

I will try to reproduce this as well.

[Sean Proctor]

Not yet. I can make one. Just wanted to verify first that I shouldn't be experiencing this.

[Arkadii Ivanov]

This shouldn't be like this.

[Sean Proctor]

I made a very simple example... and it doesn't reproduce the issue. Still investigating.

[Arkadii Ivanov]

Yup, I also couldn't reproduce the issue. Here is my test.

Untitled.kt

[Arkadii Ivanov]

I would say, make sure you never pass the parent's ComponentContext to a child component. Though, this would also prevent the instance from destroying, when the component is removed.

[Arkadii Ivanov]

I'm going to release the next alpha version by EOW. Would be really nice to find out whether this is a bug or not. I could fix it if needed.

[Sean Proctor]

I am so sorry. I should've put some debugging inside the getOrCreate lambda. My factory was accidentally returning the same data.

[Arkadii Ivanov]

Oh, no worries! Glad this is not a bug. 😅

[Sean Proctor]

Alright, all fixed on my end. That was a bit embarrassing. Thanks for your patience with me. There was an accidental annotation causing my state for one component to be reused.

[electrolobzik]

@Arkadii Ivanov Just as an idea. What if add some sort of global component registry and when new component is created check in runtime and throw exception in the case when this new component uses exactly the same component context as some of already present components. This check can be switchable, but in the most apps it will prevent accidental passing of the parent context. Since this can silently break a lot of things I suppose this check can make sense.

[Arkadii Ivanov]

@electrolobzik this wasn't an issue in this particular case. But indeed, the idea is nice. However, I don't see any ways of how it could be implemented. I.e. I don't see how Decompose can check if ComponentContext is reused. Because reusing is passing the parent ComponentContext via constructor instead of the provided one. I thought about writing a lint check that would trigger if the ComponentContext provided by the factory function is unused. But there are valid use cases when your child component doesn't require any context at all. Plus there are permanent child components where we should use childContext.

[electrolobzik]

@Arkadii Ivanov the downside of total use of composition instead of inheritance. surprisingly! 😄 the only way I see is to force all components implement some Component interface which will have something like val componentContext: ComponentContext in this case when the component is created by any fabric method the base implementation of that method can save/check the context (also removing the contexts of dead components via subscription to their lifecycles). This is not ideal for sure, but at least it will force some rules, required for a correct work. P.S. In my case I already have every component implementing ComponentContext by context so it probably will not change much in the client code. So the only change for the users will be obligatory use of T: ComponentContext instead of just <T> in all decompose methods.

[electrolobzik]

Also this probably can be just relaxed for <T: LifecycleOwner> because as I understand the lifecycle is the main point where it is important for child components to have child context.

[electrolobzik]

For users, who don’t need lifecycle for some reason there could be separate set of methods with old signature. I don’t know if it is even the case for anybody.

[electrolobzik]

I don’t know if this is worth the efforts though 😀

[Arkadii Ivanov]

Interesting idea, thanks! 🙂 I've been thinking about creating a compiler plugin that would emit a diagnostic error when it detects the following condition:

A Component is a class that implements ComponentContext. Detect cases when ComponentContext is taken from a constructor property of one Component (or as this) and passed to another Component's constructor.

[Arkadii Ivanov]

Also not ideal, won't detect if assisted inject is used.

[Arkadii Ivanov]

I'll keep thinking about it. Let's continue this discussion in a separate thread.