[WC-3163] Allow treenode to have self reference association for infinite level of children#2056
[WC-3163] Allow treenode to have self reference association for infinite level of children#2056
Conversation
gjulivan
changed the title
gjulivan
force-pushed
the
treenode-recursive
branch
5 times, most recently
from
123cfa3 to
5b56842
Compare
gjulivan
force-pushed
the
treenode-recursive
branch
2 times, most recently
from
0c323c4 to
440a0b9
Compare
|
|
||
| // retrieve new datasource for array of items | ||
| // this is used for checking children's of children (grandchildren) when a branch is expanded | ||
| const filterContents = useCallback( |
There was a problem hiding this comment.
Looks like this can be inlined into filterContent
There was a problem hiding this comment.
Or better be a helper function that gets everything as an argument.
There was a problem hiding this comment.
updated as inline code
| // ignore "datasource" update triggering fetchChildren again | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
We don't use anything except setFilter, maybe we can keep and check reference to it instead of ignoring whole datasource. This way we are safe if reference ever changes but don't get triggered on any datasource update.
There was a problem hiding this comment.
I expect // eslint-disable-next-line should go now.
| if (isInfiniteTreeNodesEnabled && fetchingItem.current === undefined) { | ||
| resolvePromise.current = resolve; | ||
| if (Array.isArray(item)) { | ||
| fetchingItem.current = item[0]; | ||
| } else { | ||
| fetchingItem.current = item; | ||
| } | ||
| datasource.setFilter(filterContent(item)); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Is it going to be a never-resolving promise if a fetch is already in progress?
There was a problem hiding this comment.
it will resolved when datasource.status === ValueStatus.Available
There was a problem hiding this comment.
If a fetch is already in progress and not finished yet and a fetch is called again, then fetchingItem.current === undefined is false (because previous didn't resolve) so the current promise will hang forever because resolve callback never assigned anywhere and wont be called.
r0b1n
left a comment
r0b1n
left a comment
There was a problem hiding this comment.
I would try decoupling the fetching/filtering logic from the react part as this if so hard to follow what is happening and makes complexity of react lifecycle add up with lifecycle of data sources.
| ); | ||
|
|
||
| // retrieve new datasource for array of items | ||
| // this is used for checking children's of children (grandchildren) when a branch is expanded |
There was a problem hiding this comment.
Why is so. Aren't we already holding all the previous levels already when we expand one level deeper?
There was a problem hiding this comment.
this is needed to "automatically" find out if a children is "hasChildren" and expandable, or not,
thus we can hide the property "hasChildren" in infinite treenode.
gjulivan
force-pushed
the
treenode-recursive
branch
from
440a0b9 to
101753b
Compare
gjulivan
force-pushed
the
treenode-recursive
branch
from
52a36f2 to
073547d
Compare
2 participants
Added parent association property to allow treenode to have infinite levels of children by setting up property to a self reference association.