Skip to content

fix: add type checking for optional properties#53

Merged
mttrbrts merged 1 commit intoaccordproject:mainfrom
Shubh-Raj:shubh-raj/fix/optional-property-typechecking
Mar 17, 2026
Merged

fix: add type checking for optional properties#53
mttrbrts merged 1 commit intoaccordproject:mainfrom
Shubh-Raj:shubh-raj/fix/optional-property-typechecking

Conversation

@Shubh-Raj
Copy link
Copy Markdown
Contributor

Description

Add compile-time type checking for optional properties in TemplateMark templates. When an optional property is used without a guard ({{#optional}}, {{#if}}, or {{#with}}), the type-checker now throws an error at compile time instead of causing a runtime failure.

Before: Runtime error - "No values found for path '$['last']'"
After: Compile-time error - "Optional properties used without guards: last"

Changes

  • Added validation in checkTypes() to detect unguarded optional property access
  • Uses path-based scoping so guards only apply to descendant nodes
  • Throws proper Error object with .errors property for structured access
  • Added test case test/templates/bad/optional-noguard/

Testing

All 26 TemplateMarkInterpreter tests pass. All 33 snapshots pass.

Related Issues

Fixes accordproject/template-playground#11

@Shubh-Raj Shubh-Raj force-pushed the shubh-raj/fix/optional-property-typechecking branch from cb71565 to dd49484 Compare January 2, 2026 06:09
@mttrbrts mttrbrts requested review from Copilot and dselman and removed request for Copilot January 8, 2026 11:14
Comment thread src/TemplateMarkInterpreter.ts Outdated
if (OPTIONAL_DEFINITION_RE.test(node.$class) ||
CONDITIONAL_DEFINITION_RE.test(node.$class) ||
WITH_DEFINITION_RE.test(node.$class)) {
if (node.name) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Under what scenario does a node not have a name property? If there is no scenario, and we have checks elsewhere, let's remove this condition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right — there's no such scenario. name is a required field on IElementDefinition, which IOptionalDefinition, IConditionalDefinition, and IWithDefinition all extend. Removed the unnecessary check in 3f9a552.

Signed-off-by: Shubh-Raj <shubhraj625@gmail.com>
@Shubh-Raj Shubh-Raj force-pushed the shubh-raj/fix/optional-property-typechecking branch from dd49484 to 3f9a552 Compare January 8, 2026 13:15
@Shubh-Raj
Copy link
Copy Markdown
Contributor Author

Following up.

@mttrbrts mttrbrts merged commit d25a21a into accordproject:main Mar 17, 2026
2 checks passed
AbdulRahmanAzam added a commit to AbdulRahmanAzam/template-engine that referenced this pull request Mar 31, 2026
…emplate

The `age` property is marked as `optional` in the Concerto model but was
used directly as `{{age}}` without an `{{#optional}}` guard. After PR accordproject#53
introduced stricter compile-time checks for optional properties, this
causes the test to fail with: "Optional properties used without guards: age"

Wraps the bare `{{age}}` reference in `{{#optional age}}{{this}}{{/optional}}`
to align with the new validation rules.

Fixes accordproject#112
mttrbrts added a commit that referenced this pull request Apr 12, 2026
…133)

* fix: wrap optional `age` property in guard block in optional-nested template

The `age` property is marked as `optional` in the Concerto model but was
used directly as `{{age}}` without an `{{#optional}}` guard. After PR #53
introduced stricter compile-time checks for optional properties, this
causes the test to fail with: "Optional properties used without guards: age"

Wraps the bare `{{age}}` reference in `{{#optional age}}{{this}}{{/optional}}`
to align with the new validation rules.

Fixes #112

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* test: update snapshot

Signed-off-by: Matt Roberts <code@rbrts.uk>

* test: update snapshot

Signed-off-by: Matt Roberts <code@rbrts.uk>

---------

Signed-off-by: Matt Roberts <code@rbrts.uk>
Co-authored-by: Matt Roberts <7544022+mttrbrts@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Matt Roberts <code@rbrts.uk>
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.

Type checker doesn't detect usage of optional properties

2 participants