Fixed
Status Update
No update yet.
Comments
ma...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
https://android-review.googlesource.com/1123258
https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
ma...@google.com
ma...@gmail.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
https://android-review.googlesource.com/1288456
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
an...@google.com
Hey! Yes, in future Lazy{Column,Row}For will use Lazy{Column,Row} internally. But I don't think it solves the key issue in any way. Could you please elaborate on your idea?
ma...@gmail.com
As far as I have seen Lazy{Column,Row} don't set the key anywhere, so it is possible to set a custom key manually. Or am I missing something?
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit aafeb1b7807ed6a353896480d25c60d18e85475b
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Wed Jan 27 18:09:39 2021
Add custom keys support for LazyColumn/LazyRow
It allows smarter reordering as we match elements by key, not by index.
Example: we had elements 0, 1, 2 and all of the elements did have some local state with use of remember {}. Without custom keys when we remove an element 1 the list is now 0, 2. But the LazyColumn operates with indexes in the list, so the remembered values which were previously associated with item 1 will now be given for the item 2.
But if we provide custom keys like this:
LazyColumn {
items(list, key = { it }) { ... }
}
We can match the composition state of the item content by these keys, not indexes. This means in the example above the state associated with 2 would still be used even when we removed the item 1. The same would happen if we just reorder items.
We will also use this key as a key for saved instance state mechanism which means that when you use rememberSaveable { } inside the item's content it would be automatically restored when the activity would be recreated after screen rotation of if you navigate back to the screen with the list when using the compose-navigation library. The downside of this is that you have to only use types which can be stored inside the android.os.Bundle on Android (On Desktop there are no restrictions). But what if your custom class is not supported by Bundle? Always try to use the whole item object as a key but something like an id of it. In most cases you have String/Long/Int id which represents the element so pass it as a key:
LazyColumn {
items(users, key = { user ->user.id }) { ... }
}
Relnote: "Custom keys support for LazyColumn/LazyRow was added. This allows us to smarter handle items reordering. So the state you stored in remember {} blocks will move together with the item when you reorder elements or removed the item from the middle.
LazyColumn {
items(users, key = { user ->user.id }) { ... }
}"
Fixes: 164901852
Test: LazyCustomKeysTest, LazyItemStateRestoration
Change-Id: Ia50ef7cd8f47ab87d76edc4e0691199ce426729d
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/ListDemos.kt
A compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/LazyCustomKeysTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/LazyItemStateRestoration.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/CachingItemContentFactory.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyDsl.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyFor.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyList.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListItemInfo.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyMeasuredItem.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyMeasuredItemProvider.kt
M compose/runtime/runtime-saveable/src/commonMain/kotlin/androidx/compose/runtime/saveable/SaveableStateHolder.kt
https://android-review.googlesource.com/1561984
Branch: androidx-main
commit aafeb1b7807ed6a353896480d25c60d18e85475b
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Wed Jan 27 18:09:39 2021
Add custom keys support for LazyColumn/LazyRow
It allows smarter reordering as we match elements by key, not by index.
Example: we had elements 0, 1, 2 and all of the elements did have some local state with use of remember {}. Without custom keys when we remove an element 1 the list is now 0, 2. But the LazyColumn operates with indexes in the list, so the remembered values which were previously associated with item 1 will now be given for the item 2.
But if we provide custom keys like this:
LazyColumn {
items(list, key = { it }) { ... }
}
We can match the composition state of the item content by these keys, not indexes. This means in the example above the state associated with 2 would still be used even when we removed the item 1. The same would happen if we just reorder items.
We will also use this key as a key for saved instance state mechanism which means that when you use rememberSaveable { } inside the item's content it would be automatically restored when the activity would be recreated after screen rotation of if you navigate back to the screen with the list when using the compose-navigation library. The downside of this is that you have to only use types which can be stored inside the android.os.Bundle on Android (On Desktop there are no restrictions). But what if your custom class is not supported by Bundle? Always try to use the whole item object as a key but something like an id of it. In most cases you have String/Long/Int id which represents the element so pass it as a key:
LazyColumn {
items(users, key = { user ->
}
Relnote: "Custom keys support for LazyColumn/LazyRow was added. This allows us to smarter handle items reordering. So the state you stored in remember {} blocks will move together with the item when you reorder elements or removed the item from the middle.
LazyColumn {
items(users, key = { user ->
}"
Fixes: 164901852
Test: LazyCustomKeysTest, LazyItemStateRestoration
Change-Id: Ia50ef7cd8f47ab87d76edc4e0691199ce426729d
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/ListDemos.kt
A compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/LazyCustomKeysTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/LazyItemStateRestoration.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/CachingItemContentFactory.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyDsl.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyFor.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyList.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListItemInfo.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyMeasuredItem.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyMeasuredItemProvider.kt
M compose/runtime/runtime-saveable/src/commonMain/kotlin/androidx/compose/runtime/saveable/SaveableStateHolder.kt
Description
Issue
Currently, each child of the
LazyColumnForcomposable is implicitlykeycontaining the index of the item. Because of this if the order of the items is changed, the children, that got reordered, are needlessly recomposed (this is because the value of the keys have changed). In the extreme case, when an item is inserted at the beginning of the list, the entire subtree ofLazyColumnForget recomposed.Motivation
Oftentimes the items that are rendered in these lazy composables, have unique ids. Thus, it would be useful if there was a way of specifying the id of the item as a key.
Possible solution
I have thought of several solutions:
Option 1 would function similarly to how it does in React. Not specifying a key could be a warning.
Personally, I like option 2 better. I am not sure how this parameter could be named, but its type could simply be:
(Int, T) -> AnyIntis the index of the itemTis the type of the itemThe default value for the parameter could be defined as
{ i, _ -> i }. This way, when the parameter is not specifiedLazyColumnFor(orLazyRowFor) would function exactly as it does now.Also maybe a better type signature could be
T.(Int) -> Any, this way if the user wants to use an id of the item as a key, the function would be as simple as{ id }instead of{ _, item -> item.id }.