-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat(angular): improvements to angularReactivityFeature, try to remove proxies #5921
Draft
riccardoperra
wants to merge
13
commits into
alpha
Choose a base branch
from
feat/alpha_angular_granular_signal
base: alpha
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
View your CI Pipeline Execution ↗ for commit 810c37a.
☁️ Nx Cloud last updated this comment at |
01cf821
to
961a7df
Compare
- rename `_setRootNotifier` to `_setTableNotifier` - add strict typings to `toComputed` - add test case for reactivity utils
…skip custom fn from table options
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Those are some experiments with
angularReactivityFeature
. The goal of this pr is to remove the proxies and make the table reactive with signals directly via a custom feature.createAngularTable/injectTable return value is currently a proxy into a proxy :
table
properties and make them reactive when the table signal changesUsing
proxifyTable
doesn't directly lead in issues after the new FlexRender implementation, but the returned table it's just a different object compared to the one obtained via (header|cell).getContext(), which the latter is not reactive.Thanks to table features added in v9, we could move away from
proxyTable
by just overriding table/cell/header/column/row props recursively viaTableFeature
interface.This allows to make all properties reactive via the same logic used by the table proxy right now using
toComputed
.Overriding all properties can be costly because if there are a lot of elements, a potential reactive signal would be created for each property of each header/row/column/cell. To work around this, we can disable reactive properties via table options (name to be decided).
skipBaseProperties
now check via string indexs instead of endsWith/startsWith to improve performance a little when there are many rowsenableExperimentalReactivity
with a newreactivity
object that allows to make reactive properties based on the type of objectsetReactiveProp
will now override with a getter that will create the computed lazily. This may improve performance a bit since previously a computed was created synchronously, while now it is created only once the getter is invoked for the first timetable()
to handle the entire table as a signal, you now need to calltable.get()
// todo
Still in draft, so there may be changes about this.
FlexRender now relies on
ngDoCheck
to check if the content is dirtyI introduced a new input (named
flexRenderNotifier
, note: tbd) that allows to change this behavior. This can be used to run view checks introduced in the previous release only when the state of the table changes, instead of every time angular determine the view is dirty.Benchmarks
On average, no reactivity takes about 34.11% less time than enabling all reactivity flags