-
Notifications
You must be signed in to change notification settings - Fork 24.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(ivy): replace LNode.dynamicLContainerNode with flat LContainers #26407
Conversation
You can preview 60f778d at https://pr26407-60f778d.ngbuilds.io/. |
60f778d
to
9409ea3
Compare
* @param value The initial value in `LViewData` | ||
*/ | ||
export function readElementValue(value: LElementNode | StylingContext | LContainer): LElementNode { | ||
if (!Array.isArray(value)) return value; // Regular LNode is stored here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this format more readable, would you mind changing it:
if (Array.isArray(value)) {
if (typeof value[ACTIVE_INDEX] === 'number') {
// This is an LContainer. It may also have a styling context.
value = value[HOST_NATIVE] as LElementNode | StylingContext;
return Array.isArray(value) ? value[StylingIndex.ElementPosition] ! : value;
} else {
// This is a StylingContext, which stores the element node at 0.
return value[StylingIndex.ElementPosition] as LElementNode;
}
} else {
return value; // Regular LNode is stored here
}
- Early returns confuse me.
- Not clear that value is array after the guard.
You can preview 085efa4 at https://pr26407-085efa4.ngbuilds.io/. |
You can preview d979871 at https://pr26407-d979871.ngbuilds.io/. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Previously, nodes with containers were stored flat in
LViewData
, so it was necessary to readLNode.data
to reach theLContainer
instance. This PR inverts this relationship, so we are now storingLContainer
instances flat inLViewData
, withLContainer[HOST_NATIVE]
storing the host nodes. This allows us to removeLNode.dynamicLContainerNode
andTNode.dynamicContainerNode
, and is the first step toward removingLNode.data
.Still TODO:
LNode.data
for component and view nodesLNode.native