Evaluating expressions only as-needed #3443
Open
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.
Description
This change rewrites the
useNodeItem()
hook and rewrites every other piece of code that relied onnodeData.item
. Instead of evaluating expressions in the node hierarchy and storing them inNodesContext
, we'll now evaluate them whenuseNodeItem()
is called.This should bring lower memory usage, and overall better performance, but it's still not optimal. Wherever only a part of the item is selected (or actually used), all properties inside will be evaluated anyway (according to the
evalExpressions()
function in the componentindex.tsx
file). In the future I want to evaluate each property when needed.A few things to note here:
AlertOnChange
plugin, as it's the only one to implement expression-evaluation in it. It's simpler to just evaluate these in each component that needs them.children
(or similar properties) will now transparently use that property for listing out children, instead of having an 'internal' type that has gone throughitemFactory()
of some plugin first. This also means that we have to filter out children that are not supposed to render there (according to theirCompCapabilities
, such asrenderInAccordion: boolean
). I made a new hook for this, calleduseHasCapability()
.children
now have base component ids in them (not indexed ones), I made a hook calleduseIndexedId()
to transform them. However, there's starting to be quite an overlap here with the (more complex) functionality around the existinguseIndexedComponentIds()
hook. However, that hook implements some useful functionality to check that a component id actually exists and is valid in that context, so removing it (or simplifying it) would break shared expression tests. I made a note about the similarity in a doc comment, but sometime in the future it would be nice to not have to deal with all this.Related Issue(s)
Verification/QA
kind/*
andbackport*
label to this PR for proper release notes grouping…n nodeData.item. This breaks stuff, but I'll fix that later - it's easier to figure out what I need to fix when TypeScript tells me what it is.