feat(ivy): support deep queries through view boundaries #21700
Conversation
pkozlowski-opensource
force-pushed
the
queries_in_embed_views_with_predicate
branch
from
6b8c4b8
to
3e026d8
Compare
pkozlowski-opensource
added
the
target: major
packages/core/src/render3/query.ts
Outdated
| @@ -97,17 +97,75 @@ export class LQuery_ implements LQuery { | |||
| } | |||
| } | |||
|
|
|||
| container(): LQuery|null { | |||
| return this._clonePredicates((predicate: QueryPredicate<any>) => { | |||
There was a problem hiding this comment.
could we solve it without closer (perf reasons)?
There was a problem hiding this comment.
After spending quite a but of time on it I'm not sure I can think of anything better. I'm at a bottom of a rabbit hole here and if there is a simple solution I just can't see it :-/
My current understanding is that we need to, for each container / view and each query:
- create a new array for results;
- make sure that views / nodes created in views are added to this new array.
Essentially we need to have a new combination of existing predicate + metdata + queryList with a new array. Cloning current predicate is the simplest I could think of... It might be that it is hard to see other solutions due to the current naming that we are using, so here is a doc describing proposed renaming: https://docs.google.com/document/d/1QOTC1pyy_WA4WT4J-xgLQhkG554SyXxBwyzUcXbc6h0/edit?usp=sharing
packages/core/src/render3/query.ts
Outdated
| @@ -240,6 +303,13 @@ function createPredicate<T>( | |||
| }; | |||
| } | |||
|
|
|||
| function flatten<T>(list: Array<T|T[]>): T[] { | |||
| return list.reduce((flat: any[], item: T | T[]): T[] => { | |||
There was a problem hiding this comment.
let's not use reduce and friends. Negative perf impact.
packages/core/src/render3/query.ts
Outdated
| @@ -240,6 +303,13 @@ function createPredicate<T>( | |||
| }; | |||
| } | |||
|
|
|||
| function flatten<T>(list: Array<T|T[]>): T[] { | |||
| return list.reduce((flat: any[], item: T | T[]): T[] => { | |||
| const flatItem = Array.isArray(item) ? flatten(item) : item; | |||
There was a problem hiding this comment.
recursive functions are perf issue since they prevent inlining.
There was a problem hiding this comment.
Created a non-recursive version, although I'm not sure how much better (perf-wise) this one is... Review appreciated.
pkozlowski-opensource
added
comp: ivy
and removed
area: core
pkozlowski-opensource
force-pushed
the
queries_in_embed_views_with_predicate
branch
from
3e026d8
to
0b4d8b3
Compare
pkozlowski-opensource
added
action: review
|
Hi @pkozlowski-opensource! This PR has merge conflicts due to recent upstream merges. |
pkozlowski-opensource
force-pushed
the
queries_in_embed_views_with_predicate
branch
from
0b4d8b3
to
c187560
Compare
pkozlowski-opensource
changed the title
pkozlowski-opensource
force-pushed
the
queries_in_embed_views_with_predicate
branch
from
c187560
to
064a6ef
Compare
|
@mhevery all the perf-related issues were addressed, please have another look |
mhevery
added
action: merge
|
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. |
@mhevery this is still WIP (requires rebase, proper description and more tests), but opening it early on so we can discuss data structure / impl choices.
The general idea is that both a container (
LContainer) and a view (LView) have a pointer to all queries "active" for a given container / view. When encountering a container and view instructions, we are cloningLQuerypredicates (queries) to have new place (arrays) to store matching nodes from sub-views.I feel like I've missed millions of use-cases here, but at the same time feel like I'm digging a deeper and deeper rabbit hole, so opening early PR so we can have discussion about exact data structures and algs.