Litho: Bug about @Treeprop

Created on 6 Jul 2018  Â·  11Comments  Â·  Source: facebook/litho

Version

*// Litho
implementation 'com.facebook.litho:litho-core:0.16.0'
implementation 'com.facebook.litho:litho-widget:0.16.0'
compileOnly 'com.facebook.litho:litho-annotations:0.16.0'
annotationProcessor 'com.facebook.litho:litho-processor:0.16.0'
// SoLoader
implementation 'com.facebook.soloader:soloader:0.2.0'
// For integration with Fresco
implementation('com.facebook.litho:litho-fresco:0.16.0') {
exclude group: 'com.facebook.fresco'
}
// For testing
testImplementation 'com.facebook.litho:litho-testing:0.16.0'

// Sections
implementation 'com.facebook.litho:litho-sections-core:0.16.0'
implementation 'com.facebook.litho:litho-sections-widget:0.16.0'
compileOnly 'com.facebook.litho:litho-sections-annotations:0.16.0'
annotationProcessor 'com.facebook.litho:litho-sections-processor:0.16.0'

Issues and Steps to Reproduce


When use @TreeProp with Sections.
The top component named 'A' of Sections can get property by use @TreeProp.
But the child component name 'B' of component 'A" can not get property by use @TreeProp.
**

Expected Behavior

Excepted 'B' can also get this property.

Link to Code

* https://github.com/jerboy/LithoTest2 *

All 11 comments

SectionBSpec can not get bean of MyLog from @TreeProp

Hi @jerboy, thanks for your report. I'm probably the best person to look into this but I'm on holidays till next week. I'll look into it as soon as I'm back.

@passy I could probably take a look at the weekend if you want!

@passy @pavlospt Thank you!

That would be amazing, Pavlos. :)

On Fri, 6 Jul 2018, 11:23 Pavlos-Petros Tournaris, notifications@github.com
wrote:

@passy https://github.com/passy I could probably take a look at the
weekend if you want!

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/facebook/litho/issues/396#issuecomment-402979909, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAmsj5OdMu8_Q-JAUm41T_bDkqa9LZaks5uDyyWgaJpZM4VFBYD
.

>

timezone: eu/london | utc+1
@passy

@passy After an investigation, I ended up finding that, probably SectionComponents should override canResolve() in order to return true. When ComponentLifecycle reaches this line https://github.com/facebook/litho/blob/a226ccb25d52f629e471ed05ba3034d15a541d87/litho-core/src/main/java/com/facebook/litho/ComponentLifecycle.java#L306 there are no TreeProps to get from that ComponentContext. At the time TreeProps can be found inside the scopedContext which are pulled in the context only if the Component can resolve.

I am not sure if that has some "business" logic or a spec behind it, but it could solve that issue IMO.

Looking forward to hearing if that's totally wrong since I am not that familiar with TreeProps.

Edit 1: That also happens only for SectionB because it is a child of SectionA and by that time SectionA has properly gotten TreeProps from SectionContext without needing to return true from canResolve().

Thanks for your investigation. I just checked if this still happens with 0.17.0 and it unfortunately does.

I believe your assessment is correct, Pavlos. The check for the internal component/canResolve needs to be broader. We had previously only noticed this for Wrapper components. I hope to have a fix for this at some point this week.

@passy Ohhh I love debugging :)

@passy @pavlospt Hello! Which release version will you fix this bug?

@jerboy This is already fixed in master, so if you grab a Snapshot, you'll get it. There'll also be a new stable release in the next few days.

Fixed in 31318d0a8342897ef02361d1beb9cdc0ea37b27e.

Thanks again for the report and the great repro steps!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

agrosner picture agrosner  Â·  8Comments

MichaelRocks picture MichaelRocks  Â·  7Comments

tpucci picture tpucci  Â·  4Comments

Syne10 picture Syne10  Â·  6Comments

Martin-Hogge picture Martin-Hogge  Â·  4Comments