Skip to content

Evaluating expressions only as-needed #3443

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

olemartinorg
Copy link
Contributor

Description

This change rewrites the useNodeItem() hook and rewrites every other piece of code that relied on nodeData.item. Instead of evaluating expressions in the node hierarchy and storing them in NodesContext, we'll now evaluate them when useNodeItem() 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 component index.tsx file). In the future I want to evaluate each property when needed.

A few things to note here:

  • I removed the 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.
  • All container components that have children (or similar properties) will now transparently use that property for listing out children, instead of having an 'internal' type that has gone through itemFactory() of some plugin first. This also means that we have to filter out children that are not supposed to render there (according to their CompCapabilities, such as renderInAccordion: boolean). I made a new hook for this, called useHasCapability().
  • Since all children now have base component ids in them (not indexed ones), I made a hook called useIndexedId() to transform them. However, there's starting to be quite an overlap here with the (more complex) functionality around the existing useIndexedComponentIds() 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

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* and backport* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

…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.

Ole Martin Handeland added 12 commits June 12, 2025 13:47
…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.
…relying on stored results. Still some typescript errors left.
… get the final nodeId when needed. This way the GridRowsPlugin doesn't have to do any manipulation.
…archy to do this for us (I forgot about checking child claims, so this does not properly filter out components that could not be claimed after all).
@olemartinorg olemartinorg added kind/other Pull requests containing chores/repo structure/other changes backport-ignore This PR is a new feature and should not be cherry-picked onto release branches labels Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-ignore This PR is a new feature and should not be cherry-picked onto release branches kind/other Pull requests containing chores/repo structure/other changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant