From bb25333a08c7298dd7a3940f54f9e4221163c13f Mon Sep 17 00:00:00 2001 From: Yash Goel <162511050+yashhzd@users.noreply.github.com> Date: Thu, 26 Feb 2026 07:19:36 +0530 Subject: [PATCH 1/2] fix(markdown-template): preserve parent scope in conditional and optional blocks ConditionalDefinition (#if) and OptionalDefinition (#optional) blocks with primitive types incorrectly changed the model scope to the condition property, preventing child variables from resolving against the parent class declaration. This caused 'Unknown property' errors when using named variables like {{age}} inside {{#if age}}...{{/if}} blocks. Changes: - ConditionalDefinition: pass currentModel instead of property to whenTrue children so variables resolve against the parent scope - OptionalDefinition: for primitive types, keep parent model scope instead of narrowing to the property - VariableDefinition: add type guard for {{this}} handler to prevent crash when currentModel is a ClassDeclaration Closes accordproject/template-playground#2 Signed-off-by: Yash Goel <162511050+yashhzd@users.noreply.github.com> --- packages/markdown-template/lib/TypeVisitor.js | 19 ++-- packages/markdown-template/src/TypeVisitor.js | 13 +-- .../test/TemplateMarkTransformer.js | 86 +++++++++++++++++++ 3 files changed, 105 insertions(+), 13 deletions(-) diff --git a/packages/markdown-template/lib/TypeVisitor.js b/packages/markdown-template/lib/TypeVisitor.js index 8b48c940..b7e7a2e8 100644 --- a/packages/markdown-template/lib/TypeVisitor.js +++ b/packages/markdown-template/lib/TypeVisitor.js @@ -150,7 +150,7 @@ class TypeVisitor { // of complex types using a {{this}}, then thing will be a ClassDeclaration or an // EnumDeclaration!! - if (property && property.getType) { + if (property && property.getType && typeof property.isPrimitive === 'function') { var _property$isRelations; var serializer = parameters.templateMarkModelManager.getSerializer(); thing.decorators = processDecorators(serializer, property); @@ -298,7 +298,6 @@ class TypeVisitor { case 'ConditionalDefinition': { var _property5 = currentModel.getOwnProperty(thing.name); - var _nextModel2; if (thing.name !== 'if' && !_property5) { // hack, allow the node to have the name 'if' _throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing); @@ -309,11 +308,12 @@ class TypeVisitor { // } var _serializer6 = parameters.templateMarkModelManager.getSerializer(); thing.decorators = _property5 ? processDecorators(_serializer6, _property5) : null; - _nextModel2 = _property5; + // Conditional blocks do not change scope — variables inside #if + // must resolve against the parent model, not the condition property TypeVisitor.visitChildren(this, thing, { templateMarkModelManager: parameters.templateMarkModelManager, introspector: parameters.introspector, - model: _nextModel2, + model: currentModel, kind: parameters.kind }, 'whenTrue'); TypeVisitor.visitChildren(this, thing, { @@ -327,7 +327,7 @@ class TypeVisitor { case 'OptionalDefinition': { var _property6 = currentModel.getOwnProperty(thing.name); - var _nextModel3; + var _nextModel2; if (!_property6) { _throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing); } @@ -338,15 +338,18 @@ class TypeVisitor { thing.decorators = processDecorators(_serializer7, _property6); if (_property6.isPrimitive()) { thing.elementType = _property6.getFullyQualifiedTypeName(); - _nextModel3 = _property6; + // For primitive optional properties, keep the parent model scope + // so that named variables (e.g. {{age}}) can resolve correctly. + // The property itself is passed as parentModel for {{this}} fallback. + _nextModel2 = currentModel; } else { thing.elementType = _property6.getFullyQualifiedTypeName(); - _nextModel3 = parameters.introspector.getClassDeclaration(thing.elementType); + _nextModel2 = parameters.introspector.getClassDeclaration(thing.elementType); } TypeVisitor.visitChildren(this, thing, { templateMarkModelManager: parameters.templateMarkModelManager, introspector: parameters.introspector, - model: _nextModel3, + model: _nextModel2, kind: parameters.kind }, 'whenSome'); TypeVisitor.visitChildren(this, thing, { diff --git a/packages/markdown-template/src/TypeVisitor.js b/packages/markdown-template/src/TypeVisitor.js index a5ea5b4b..d5b08a89 100644 --- a/packages/markdown-template/src/TypeVisitor.js +++ b/packages/markdown-template/src/TypeVisitor.js @@ -148,7 +148,7 @@ class TypeVisitor { // of complex types using a {{this}}, then thing will be a ClassDeclaration or an // EnumDeclaration!! - if (property && property.getType) { + if (property && property.getType && typeof property.isPrimitive === 'function') { const serializer = parameters.templateMarkModelManager.getSerializer(); thing.decorators = processDecorators(serializer,property); if (property.isTypeEnum && property.isTypeEnum()) { @@ -288,7 +288,6 @@ class TypeVisitor { break; case 'ConditionalDefinition': { const property = currentModel.getOwnProperty(thing.name); - let nextModel; if (thing.name !== 'if' && !property) { // hack, allow the node to have the name 'if' _throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing); } @@ -298,11 +297,12 @@ class TypeVisitor { // } const serializer = parameters.templateMarkModelManager.getSerializer(); thing.decorators = property ? processDecorators(serializer,property) : null; - nextModel = property; + // Conditional blocks do not change scope — variables inside #if + // must resolve against the parent model, not the condition property TypeVisitor.visitChildren(this, thing, { templateMarkModelManager:parameters.templateMarkModelManager, introspector:parameters.introspector, - model:nextModel, + model:currentModel, kind:parameters.kind }, 'whenTrue'); TypeVisitor.visitChildren(this, thing, { @@ -326,7 +326,10 @@ class TypeVisitor { thing.decorators = processDecorators(serializer,property); if (property.isPrimitive()) { thing.elementType = property.getFullyQualifiedTypeName(); - nextModel = property; + // For primitive optional properties, keep the parent model scope + // so that named variables (e.g. {{age}}) can resolve correctly. + // The property itself is passed as parentModel for {{this}} fallback. + nextModel = currentModel; } else { thing.elementType = property.getFullyQualifiedTypeName(); nextModel = parameters.introspector.getClassDeclaration(thing.elementType); diff --git a/packages/markdown-template/test/TemplateMarkTransformer.js b/packages/markdown-template/test/TemplateMarkTransformer.js index 0aa0b368..18645625 100644 --- a/packages/markdown-template/test/TemplateMarkTransformer.js +++ b/packages/markdown-template/test/TemplateMarkTransformer.js @@ -33,6 +33,24 @@ concept Thing { o String[] items }`; +const CONDITIONAL_MODEL = ` +namespace test@1.0.0 +@template +concept TemplateData { + o Integer age optional + o String name + o Boolean isActive optional +}`; + +const OPTIONAL_MODEL = ` +namespace test@1.0.0 +@template +concept TemplateData { + o Integer age optional + o String middleName optional + o Boolean active optional +}`; + describe('#TemplateMarkTransformer', () => { describe('#tokensToMarkdownTemplate', () => { it('should handle join with type, style and locale', async () => { @@ -92,4 +110,72 @@ describe('#TemplateMarkTransformer', () => { (joinNode.foo === undefined).should.be.true; }); }); + + describe('#conditional blocks with variables', () => { + it('should allow named variables inside #if blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(CONDITIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#if age}}You are {{age}} years old.{{/if}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should allow multiple variables inside #if blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(CONDITIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#if isActive}}Hello {{name}}, you are {{age}} years old.{{/if}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should reject unknown variables inside #if blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(CONDITIONAL_MODEL); + (() => transformer.fromMarkdownTemplate( + {content: '{{#if age}}You are {{unknown}} years old.{{/if}}'}, + modelManager, 'clause', {verbose: false} + )).should.throw(); + }); + }); + + describe('#optional blocks with variables', () => { + it('should allow named variables inside #optional blocks with primitive types', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#optional age}}You are {{age}} years old.{{else}}Age unknown.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should allow different variables inside #optional blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#optional middleName}}Middle name: {{middleName}}{{else}}No middle name.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should reject unknown variables inside #optional blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + (() => transformer.fromMarkdownTemplate( + {content: '{{#optional age}}You are {{unknown}} years old.{{else}}Nope.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + )).should.throw(); + }); + }); }); \ No newline at end of file From eb6904faee34de43ed0eccf5e1e001daa89d71e3 Mon Sep 17 00:00:00 2001 From: Yash Goel <162511050+yashhzd@users.noreply.github.com> Date: Sun, 15 Mar 2026 04:23:46 +0530 Subject: [PATCH 2/2] fix(markdown-template): address review feedback for scope handling - Pass currentModel to whenFalse/whenNone branches so variables inside {{else}} blocks resolve correctly instead of failing with null model - Add primitiveProperty parameter to preserve {{this}} typing inside primitive #optional blocks (prevents regression where {{this}} would resolve as parent class type instead of the optional property type) - Fix misleading comment about non-existent parentModel parameter - Add 5 new tests covering {{else}} branch variable resolution for both #if and #optional blocks, plus {{this}} in primitive optionals Signed-off-by: Yash Goel --- packages/markdown-template/lib/TypeVisitor.js | 14 ++--- packages/markdown-template/src/TypeVisitor.js | 14 ++--- .../test/TemplateMarkTransformer.js | 53 +++++++++++++++++++ 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/packages/markdown-template/lib/TypeVisitor.js b/packages/markdown-template/lib/TypeVisitor.js index b7e7a2e8..e0ebc2f0 100644 --- a/packages/markdown-template/lib/TypeVisitor.js +++ b/packages/markdown-template/lib/TypeVisitor.js @@ -146,7 +146,7 @@ class TypeVisitor { _throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing); } if (thing.name === 'this') { - var property = currentModel; // BUG... if we are iterating over an array + var property = parameters.primitiveProperty || currentModel; // BUG... if we are iterating over an array // of complex types using a {{this}}, then thing will be a ClassDeclaration or an // EnumDeclaration!! @@ -319,7 +319,7 @@ class TypeVisitor { TypeVisitor.visitChildren(this, thing, { templateMarkModelManager: parameters.templateMarkModelManager, introspector: parameters.introspector, - model: null, + model: currentModel, kind: parameters.kind }, 'whenFalse'); } @@ -338,9 +338,10 @@ class TypeVisitor { thing.decorators = processDecorators(_serializer7, _property6); if (_property6.isPrimitive()) { thing.elementType = _property6.getFullyQualifiedTypeName(); - // For primitive optional properties, keep the parent model scope - // so that named variables (e.g. {{age}}) can resolve correctly. - // The property itself is passed as parentModel for {{this}} fallback. + // For primitive optional properties, keep the parent model as the + // current scope so that named variables (e.g. {{age}}) resolve + // against the parent class. The property is stashed separately + // so that {{this}} can still resolve to the primitive type. _nextModel2 = currentModel; } else { thing.elementType = _property6.getFullyQualifiedTypeName(); @@ -350,12 +351,13 @@ class TypeVisitor { templateMarkModelManager: parameters.templateMarkModelManager, introspector: parameters.introspector, model: _nextModel2, + primitiveProperty: _property6.isPrimitive() ? _property6 : null, kind: parameters.kind }, 'whenSome'); TypeVisitor.visitChildren(this, thing, { templateMarkModelManager: parameters.templateMarkModelManager, introspector: parameters.introspector, - model: null, + model: currentModel, kind: parameters.kind }, 'whenNone'); } diff --git a/packages/markdown-template/src/TypeVisitor.js b/packages/markdown-template/src/TypeVisitor.js index d5b08a89..f5da2b55 100644 --- a/packages/markdown-template/src/TypeVisitor.js +++ b/packages/markdown-template/src/TypeVisitor.js @@ -144,7 +144,7 @@ class TypeVisitor { _throwTemplateExceptionForElement('Unknown property: ' + thing.name, thing); } if (thing.name === 'this') { - const property = currentModel; // BUG... if we are iterating over an array + const property = parameters.primitiveProperty || currentModel; // BUG... if we are iterating over an array // of complex types using a {{this}}, then thing will be a ClassDeclaration or an // EnumDeclaration!! @@ -308,7 +308,7 @@ class TypeVisitor { TypeVisitor.visitChildren(this, thing, { templateMarkModelManager:parameters.templateMarkModelManager, introspector:parameters.introspector, - model:null, + model:currentModel, kind:parameters.kind }, 'whenFalse'); } @@ -326,9 +326,10 @@ class TypeVisitor { thing.decorators = processDecorators(serializer,property); if (property.isPrimitive()) { thing.elementType = property.getFullyQualifiedTypeName(); - // For primitive optional properties, keep the parent model scope - // so that named variables (e.g. {{age}}) can resolve correctly. - // The property itself is passed as parentModel for {{this}} fallback. + // For primitive optional properties, keep the parent model as the + // current scope so that named variables (e.g. {{age}}) resolve + // against the parent class. The property is stashed separately + // so that {{this}} can still resolve to the primitive type. nextModel = currentModel; } else { thing.elementType = property.getFullyQualifiedTypeName(); @@ -338,12 +339,13 @@ class TypeVisitor { templateMarkModelManager:parameters.templateMarkModelManager, introspector:parameters.introspector, model:nextModel, + primitiveProperty: property.isPrimitive() ? property : null, kind:parameters.kind }, 'whenSome'); TypeVisitor.visitChildren(this, thing, { templateMarkModelManager:parameters.templateMarkModelManager, introspector:parameters.introspector, - model:null, + model:currentModel, kind:parameters.kind }, 'whenNone'); } diff --git a/packages/markdown-template/test/TemplateMarkTransformer.js b/packages/markdown-template/test/TemplateMarkTransformer.js index 18645625..b944bedd 100644 --- a/packages/markdown-template/test/TemplateMarkTransformer.js +++ b/packages/markdown-template/test/TemplateMarkTransformer.js @@ -143,6 +143,27 @@ describe('#TemplateMarkTransformer', () => { modelManager, 'clause', {verbose: false} )).should.throw(); }); + + it('should allow variables inside {{else}} branch of #if blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(CONDITIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#if isActive}}Hello {{name}}.{{else}}Goodbye {{name}}.{{/if}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should reject unknown variables inside {{else}} branch of #if blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(CONDITIONAL_MODEL); + (() => transformer.fromMarkdownTemplate( + {content: '{{#if isActive}}Hello {{name}}.{{else}}Goodbye {{unknown}}.{{/if}}'}, + modelManager, 'clause', {verbose: false} + )).should.throw(); + }); }); describe('#optional blocks with variables', () => { @@ -177,5 +198,37 @@ describe('#TemplateMarkTransformer', () => { modelManager, 'clause', {verbose: false} )).should.throw(); }); + + it('should allow variables inside {{else}} branch of #optional blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#optional age}}You are {{age}} years old.{{else}}Active: {{active}}.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); + + it('should reject unknown variables inside {{else}} branch of #optional blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + (() => transformer.fromMarkdownTemplate( + {content: '{{#optional age}}You are {{age}}.{{else}}Unknown: {{bogus}}.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + )).should.throw(); + }); + + it('should allow {{this}} inside primitive #optional blocks', async () => { + const transformer = new TemplateMarkTransformer(); + const modelManager = new ModelManager(); + modelManager.addCTOModel(OPTIONAL_MODEL); + const result = transformer.fromMarkdownTemplate( + {content: '{{#optional age}}Age is {{this}}.{{else}}No age.{{/optional}}'}, + modelManager, 'clause', {verbose: false} + ); + result.should.not.be.null; + }); }); }); \ No newline at end of file