Repository navigation
(Insert|Move|Remove)ChildRenderObject methods lack parity #63269
Description
Problem Statement
Currently, these methods have the following signatures:
void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot);
void moveChildRenderObject(covariant RenderObject child, covariant dynamic slot);
void removeChildRenderObject(covariant RenderObject child);Not having the old slot in the cases of move and remove makes it difficult for render objects to efficiently locate the child in question without having to scan for it in order to move or remove it. This in turn has yielded a code smell of the childToSlot mapping pattern that has been copied in several places in the framework:
Alternative
Since render objects have various different child models (single child list, multiple child lists, named children), it would be very useful if the signatures were as follows:
void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot);
void moveChildRenderObject(covariant RenderObject child, covariant dynamic oldSlot, covariant dynamic newSlot);
void removeChildRenderObject(covariant RenderObject child, covariant dynamic slot);The difference is the addition of the old slot for move and remove. Passing this information would allow the render object to efficiently locate the child in question without having to scan for it in order to move or remove it.
Breaking Change Mitigation
The proposed API above has the problem that it's a hard breaking change for any code that extends RenderObjectElement -- so basically any custom render objects. That's a big enough breaking change that it's basically a non-starter as written above.
So I propose that we transition to new API methods with the desired signature -- with a soft transition and deprecation period.
// The new methods, eventually to be made abstract, are initially (during the soft
// transition period) stub impls that just delegate to the deprecated methods.
@protected
void insertRenderObjectChild(covariant RenderObject child, covariant dynamic slot) {
insertChildRenderObject(child, slot);
}
@protected
void moveRenderObjectChild(covariant RenderObject child, covariant dynamic oldSlot, covariant dynamic newSlot) {
moveChildRenderObject(child, newSlot);
}
@protected
void removeRenderObjectChild(covariant RenderObject child, covariant dynamic slot) {
removeChildRenderObject(child);
}
// The deprecated methods cease to be abstract, and instead provide stub impls
// that throw, pointing the user to implement the new signatures.
@Deprecated('Override insertRenderObjectChild instead')
void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot) {
assert(() {
// Throw an error instructing them to implement insertRenderObjectChild()
}());
}
@Deprecated('Override moveRenderObjectChild instead')
void moveChildRenderObject(covariant RenderObject child, covariant dynamic slot) {
assert(() {
// Throw an error instructing them to implement moveRenderObjectChild()
}());
}
@Deprecated('Override removeRenderObjectChild instead')
void removeChildRenderObject(covariant RenderObject child) {
assert(() {
// Throw an error instructing them to implement removeRenderObjectChild()
}());
}Other Info
It's worth noting that the framework has this extra slot information in hand - wiring it up would be trivial:
Activity
tvolkert commentedon Aug 8, 2020
This will require dart-lang/sdk#42991
Hixie commentedon Aug 8, 2020
SGTM. Make it so.
We could use @mustCallSuper instead of (as well as) @deprecated, so we wouldn't need to wait until dart-lang/sdk#42991.
cc @goderbauer
goderbauer commentedon Aug 10, 2020
SGTM as well.
(insert|move|remove)ChildRenderObject Deprecation: Step 1 (#64189)
Remove remaining uses of `childToSlot` (#64273)
tvolkert commentedon Aug 21, 2020
The initial part of this is done now. All that's left to do is to remove the deprecated methods after the change has been in stable. I'm closing this, since there's no need to leave the issue open to track the final removal/cleanup
(insert|move|remove)ChildRenderObject Deprecation: Step 1 (flutter#64189
Remove remaining uses of `childToSlot` (flutter#64273)
(insert|move|remove)ChildRenderObject Deprecation: Step 1 (flutter#64189
Remove remaining uses of `childToSlot` (flutter#64273)
github-actions commentedon Aug 13, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.