-
Notifications
You must be signed in to change notification settings - Fork 6.8k
refactor(tree): avoid improper query usage #16540
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
Conversation
4eb9921
to
7086935
Compare
7086935
to
2092d2b
Compare
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.
LGTM
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.
LGTM
Copying from slack: A bunch of targets are failing in Google on this because:
|
2092d2b
to
fba7973
Compare
The way things are set up at the moment we have a `CdkNestedTreeNode` which uses a `ContentChildren` to find a `CdkTreeNodeOutlet` so that it knows where to render its content. On top of this we have a `MatNestedTreeNode` which extends `CdkNestedTreeNode` and defines it's own query at the same property, but looking for a `MatTreeNodeOutlet`. With this setup it means that both `CdkNestedTreeNode` and `MatNestedTreeNode` are trying to write to the same `nodeOutlet` property and it's by pure coincidence that it works. This is very fragile and it isn't guaranteed to work in future versions of the framework. These changes fix the issue by removing the query from the `MatNestedTreeNode` and providing the `MatTreeNodeOutlet` as a `CdkTreeNodeOutlet` so that the query on the `CdkNestedTreeNode` can pick it up.
fba7973
to
cf4f46c
Compare
Updated to remove the mixin class. |
The way things are set up at the moment we have a `CdkNestedTreeNode` which uses a `ContentChildren` to find a `CdkTreeNodeOutlet` so that it knows where to render its content. On top of this we have a `MatNestedTreeNode` which extends `CdkNestedTreeNode` and defines it's own query at the same property, but looking for a `MatTreeNodeOutlet`. With this setup it means that both `CdkNestedTreeNode` and `MatNestedTreeNode` are trying to write to the same `nodeOutlet` property and it's by pure coincidence that it works. This is very fragile and it isn't guaranteed to work in future versions of the framework. These changes fix the issue by removing the query from the `MatNestedTreeNode` and providing the `MatTreeNodeOutlet` as a `CdkTreeNodeOutlet` so that the query on the `CdkNestedTreeNode` can pick it up.
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. |
The way things are set up at the moment we have a
CdkNestedTreeNode
which uses aContentChildren
to find aCdkTreeNodeOutlet
so that it knows where to render its content. On top of this we have aMatNestedTreeNode
which extendsCdkNestedTreeNode
and defines it's own query at the same property, but looking for aMatTreeNodeOutlet
. With this setup it means that bothCdkNestedTreeNode
andMatNestedTreeNode
are trying to write to the samenodeOutlet
property and it's by pure coincidence that it works. This is very fragile and it isn't guaranteed to work in future versions of the framework.These changes fix the issue by removing the query from the
MatNestedTreeNode
and providing theMatTreeNodeOutlet
as aCdkTreeNodeOutlet
so that the query on theCdkNestedTreeNode
can pick it up.cc @pkozlowski-opensource