fix: correctly evaluate clause and conditional expressions#118
Open
salama968 wants to merge 2 commits intoaccordproject:mainfrom
Open
Conversation
Signed-off-by: ahmedsalama <slamah968@gmail.com>
There was a problem hiding this comment.
Pull request overview
Fixes evaluation of condition="..." for {{#if}} and {{#clause}} constructs in the TemplateMark interpreter so that explicit conditions are evaluated correctly (including proper handling of "false" results), and adds regression fixtures.
Changes:
- Parse JSON-stringified condition results instead of using truthiness coercion for
ConditionalDefinitionandClauseDefinition. - Reorder clause handling to evaluate explicit
context.conditionbefore the implicit “undefined/null” clause-skip check. - Add three new “good template” fixtures and update Jest snapshots to cover clause condition scenarios.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/TemplateMarkInterpreter.ts | Fixes clause/conditional condition evaluation precedence and boolean parsing. |
| src/runtime/declarations.ts | Updates generated base64-encoded runtime typings (Smart Legal Contract). |
| test/templates/good/clause-condition-optional/template.md | New fixture: clause condition with missing optional field. |
| test/templates/good/clause-condition-optional/model.cto | New fixture model for optional clause condition test. |
| test/templates/good/clause-condition-optional/data.json | New fixture data (optional field missing). |
| test/templates/good/clause-condition-optional-present/template.md | New fixture: clause condition with optional field present. |
| test/templates/good/clause-condition-optional-present/model.cto | New fixture model for optional-present test. |
| test/templates/good/clause-condition-optional-present/data.json | New fixture data (optional field present). |
| test/templates/good/clause-condition-false/template.md | New fixture: clause condition always false. |
| test/templates/good/clause-condition-false/model.cto | New fixture model for always-false test. |
| test/templates/good/clause-condition-false/data.json | New fixture data for always-false test. |
| test/snapshots/TemplateMarkInterpreter.test.ts.snap | Snapshot updates for new fixtures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: ahmedsalama <slamah968@gmail.com>
This file contains hidden or 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
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.
Closes #117
This PR resolves issue #117 by fixing two critical bugs in TemplateMarkInterpreter.ts that prevented {{#clause condition}} statement from evaluating correctly.
changes
Fixed Evaluation Precedence: Reordered the logic in clause definition handling so that explicit user conditions (context.condition) are evaluated before the implicit undefined field checks. This ensures the user's condition logic is no longer bypassed when optional fields are missing.
Fixed Boolean Parsing: Replaced the flawed !!result truthiness check with JSON.parse(resultStr). Previously, a condition returning false was stringified as "false", which incorrectly evaluated to true. It now correctly parses to a boolean false.
Tests
Added 3 new Jest test cases (clause-condition-*) to explicitly verify:
Clauses with conditions properly hide when optional fields are missing.
Clauses with conditions properly render when optional fields are present.
Clauses with explicit return false conditions do not render, even if the data field exists (verifying the boolean parsing fix).
related issues