feat: Support objects in frontend expressions - #18982
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesObject Expression Type
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0cc8994 to
fe6d2e2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…essions # Conflicts: # src/App/frontend/monorepo-changed-paths.txt
…ns/component/lookup-list.json
Co-authored-by: Ole Martin Handeland <github@olemartin.org>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App/frontend/src/codegen/dataTypes/GenerateExpressionOr.ts (1)
7-24:⚠️ Potential issue | 🟠 MajorExtend
TypeMapto supportExprVal.Object.The lookup entries in
toTsMapandtoSchemaMaphave been correctly updated forExprVal.Object, butTypeMap(lines 27–33) still only handles Boolean, Number, and String. This meansGenerateExpressionOr<ExprVal.Object>incorrectly resolves tonever, breaking the type contract even though no current code instantiates this variant. Add the missing conditional branch toTypeMapbefore this generator is used with Object types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App/frontend/src/codegen/dataTypes/GenerateExpressionOr.ts` around lines 7 - 24, The TypeMap type definition does not include a conditional branch for ExprVal.Object, which causes GenerateExpressionOr to incorrectly resolve to never for Object types even though toTsMap and toSchemaMap have been correctly updated with Object entries. Add a conditional branch to TypeMap that mirrors the pattern used for ExprVal.Boolean, ExprVal.Number, and ExprVal.String to properly handle the ExprVal.Object case and return the correct type mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App/frontend/src/features/expressions/validation.ts`:
- Around line 349-351: The isValidObject function in the validation.ts file uses
typeof value === 'object' which returns true for arrays and class instances like
Date, causing arrays and non-plain objects to be incorrectly accepted as valid
objects. Fix this by adding a check to exclude arrays using Array.isArray(value)
and to ensure the value is a plain object rather than a class instance. Add
!Array.isArray(value) to the condition, and additionally verify that
Object.getPrototypeOf(value) === Object.prototype to exclude class instances and
ensure only plain objects are accepted as valid objects.
---
Outside diff comments:
In `@src/App/frontend/src/codegen/dataTypes/GenerateExpressionOr.ts`:
- Around line 7-24: The TypeMap type definition does not include a conditional
branch for ExprVal.Object, which causes GenerateExpressionOr to incorrectly
resolve to never for Object types even though toTsMap and toSchemaMap have been
correctly updated with Object entries. Add a conditional branch to TypeMap that
mirrors the pattern used for ExprVal.Boolean, ExprVal.Number, and ExprVal.String
to properly handle the ExprVal.Object case and return the correct type mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c2eb29a-5c9f-4e15-b4a3-9e7a35948f15
📒 Files selected for processing (14)
src/App/frontend/schemas/json/layout/expression.schema.v1.jsonsrc/App/frontend/src/codegen/dataTypes/GenerateExpressionOr.tssrc/App/frontend/src/features/expressions/ObjectFunctionEvaluator.tssrc/App/frontend/src/features/expressions/expression-functions.tssrc/App/frontend/src/features/expressions/index.tssrc/App/frontend/src/features/expressions/shared-tests/functions/dataModel/lookup-list.jsonsrc/App/frontend/src/features/expressions/shared-tests/functions/dataModel/lookup-object.jsonsrc/App/frontend/src/features/expressions/shared-tests/functions/dataModel/object-is-null.jsonsrc/App/frontend/src/features/expressions/shared-tests/functions/list/list.jsonsrc/App/frontend/src/features/expressions/shared-tests/functions/object/object.jsonsrc/App/frontend/src/features/expressions/types.tssrc/App/frontend/src/features/expressions/validation.tssrc/App/frontend/src/utils/arrayUtils.test.tssrc/App/frontend/src/utils/arrayUtils.ts
💤 Files with no reviewable changes (1)
- src/App/frontend/src/features/expressions/shared-tests/functions/dataModel/object-is-null.json
Important
This pull request is stacked upon #18974, which should be merged first.
Description
This pull request adds support for objects in frontend expressions, along with an
objectfunction for creating an object. Here is an overview of the changes:objecttype inExprValobjectfunction inexpression-functions, using a new class namedObjectFunctionEvaluatorHere is the corresponding change in App Backend: Altinn/app-lib-dotnet#1777
Verification
Summary by CodeRabbit
object/strict-objectvalidation.object()function to construct objects from key/value pairs (with validation for even argument counts, string keys, and unique keys).any.