(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: