Skip to content

feat(form-engine): add dependency tracking for form field expressions and improve calculate expression evaluation order#654

Open
delcroip wants to merge 7 commits intoopenmrs:mainfrom
delcroip:main
Open

feat(form-engine): add dependency tracking for form field expressions and improve calculate expression evaluation order#654
delcroip wants to merge 7 commits intoopenmrs:mainfrom
delcroip:main

Conversation

@delcroip
Copy link

@delcroip delcroip commented Oct 12, 2025

Vibe coding to solve https://talk.openmrs.org/t/in-o3-forn-calculateexpression-does-not-seems-capable-of-using-other-calculateexpression/47176

  • Add acorn and acorn-walk dependencies for AST-based expression parsing
  • Implement trackFieldDependenciesFromString function in expression-runner.ts
  • Add dependency tracking for hide, disable, and readonly expressions in useEvaluateFormFieldExpressions hook
  • Add dependency tracking for answer hide and disable expressions in fieldLogic.ts
  • Refactor calculate expression evaluation in encounter-form-processor.ts to use topological sort for dependency order
  • Add utility functions: extractDependencies, buildDependencyGraph, topologicalSort
  • Add new utility files: variable-extractor.ts and variable-extractor.test.ts
  • Add test files for useEvaluateFormFieldExpressions and encounter-form-processor
  • Update utils/index.ts to export new utilities

seems to work on the browser

… and improve calculate expression evaluation order

- Add acorn and acorn-walk dependencies for AST-based expression parsing
- Implement trackFieldDependenciesFromString function in expression-runner.ts
- Add dependency tracking for hide, disable, and readonly expressions in useEvaluateFormFieldExpressions hook
- Add dependency tracking for answer hide and disable expressions in fieldLogic.ts
- Refactor calculate expression evaluation in encounter-form-processor.ts to use topological sort for dependency order
- Add utility functions: extractDependencies, buildDependencyGraph, topologicalSort
- Add new utility files: variable-extractor.ts and variable-extractor.test.ts
- Add test files for useEvaluateFormFieldExpressions and encounter-form-processor
- Update utils/index.ts to export new utilities
@samuelmale
Copy link
Member

Thank you for your contributions towards this project.

was not able yet to test it in a browser

Untested PRs cannot be merged. We also expect you to attach screenshots or a short video demonstrating how your proposed solution resolves the issue.

I was able to reproduce the issue locally, and I believe it’s possible to implement a simpler fix. The form engine already has a basic mechanism for tracking field dependencies. When a field B depends on another field A, the FE maintains a list of dependents for field A. Whenever A’s value changes, it triggers a cascade of onChange events to all its dependents. This approach is fairly simple but works well for most known use cases.

The issue:
We have two fields, A and B, where B depends on A for its value. However, A itself is initialized through a calculated expression, which means its value is resolved asynchronously. During form initialization, the EncounterFormProcessor evaluates calculated expressions sequentially but does not yet track field dependencies (this dependency tracking only begins after the form is fully initialized and the React tree is set up).

Suggested direction:
To resolve this, the EncounterFormProcessor should ensure that initial values are loaded in dependency order; that is, it should follow the dependency tree when evaluating calculated expressions during initialization.

Comment on lines +197 to +202
// Track dependencies for answer disable expressions
trackFieldDependenciesFromString(
answer.disable?.disableWhenExpression,
{ value: dependent, type: 'field' },
formFields,
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure this is still needed (was added along investigation but might have been a wrong approach)

@delcroip
Copy link
Author

delcroip commented Oct 28, 2025

Thank you for your contributions towards this project.

was not able yet to test it in a browser

Untested PRs cannot be merged. We also expect you to attach screenshots or a short video demonstrating how your proposed solution resolves the issue.

I was able to reproduce the issue locally, and I believe it’s possible to implement a simpler fix. The form engine already has a basic mechanism for tracking field dependencies. When a field B depends on another field A, the FE maintains a list of dependents for field A. Whenever A’s value changes, it triggers a cascade of onChange events to all its dependents. This approach is fairly simple but works well for most known use cases.

The issue: We have two fields, A and B, where B depends on A for its value. However, A itself is initialized through a calculated expression, which means its value is resolved asynchronously. During form initialization, the EncounterFormProcessor evaluates calculated expressions sequentially but does not yet track field dependencies (this dependency tracking only begins after the form is fully initialized and the React tree is set up).

Suggested direction: To resolve this, the EncounterFormProcessor should ensure that initial values are loaded in dependency order; that is, it should follow the dependency tree when evaluating calculated expressions during initialization.

it is a bit more profound, getValue did not work on Calculate; honestly all that code was written by AI but it does work and pass all tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants