From 208b2c5140286f0c379c98a8236af23ba33a0aa0 Mon Sep 17 00:00:00 2001 From: Mike Engel Date: Wed, 29 Nov 2023 14:11:18 +0100 Subject: [PATCH 1/4] test: add failing test --- .../macros/tests/glimmer/dependency-satisfies.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/macros/tests/glimmer/dependency-satisfies.test.ts b/packages/macros/tests/glimmer/dependency-satisfies.test.ts index e8c389dc8..80180f073 100644 --- a/packages/macros/tests/glimmer/dependency-satisfies.test.ts +++ b/packages/macros/tests/glimmer/dependency-satisfies.test.ts @@ -29,6 +29,14 @@ describe('dependency satisfies', () => { expect(result).toMatch(/@a=\{\{true\}\}/); }); + test('in branch', () => { + let result = transform( + `{{#if (macroDependencySatisfies 'qunit' '^2.8.0')}}
{{else}}{{/if}}`, + { filename } + ); + expect(result).toEqual('
'); + }); + test('emits false for out-of-range package', () => { let result = transform(`{{macroDependencySatisfies 'qunit' '^10.0.0'}}`, { filename }); expect(result).toEqual('{{false}}'); From c3880d9c9a68bbcc5f628a5ac9aa7bab9f903cb7 Mon Sep 17 00:00:00 2001 From: Mike Engel Date: Wed, 29 Nov 2023 16:01:35 +0100 Subject: [PATCH 2/4] fix: transform macroDependencySatisfies to a macroCondition --- packages/macros/src/glimmer/ast-transform.ts | 4 +++- packages/macros/tests/glimmer/dependency-satisfies.test.ts | 7 ++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/macros/src/glimmer/ast-transform.ts b/packages/macros/src/glimmer/ast-transform.ts index b7d0a3aa9..c0a29cea5 100644 --- a/packages/macros/src/glimmer/ast-transform.ts +++ b/packages/macros/src/glimmer/ast-transform.ts @@ -97,7 +97,9 @@ export function makeFirstTransform(opts: FirstTransformParams) { ); } if (node.path.original === 'macroDependencySatisfies') { - return literal(dependencySatisfies(node, opts.packageRoot, moduleName, packageCache), env.syntax.builders); + return env.syntax.builders.sexpr('macroCondition', [ + literal(dependencySatisfies(node, opts.packageRoot, moduleName, packageCache), env.syntax.builders), + ]); } }, MustacheStatement(node: any) { diff --git a/packages/macros/tests/glimmer/dependency-satisfies.test.ts b/packages/macros/tests/glimmer/dependency-satisfies.test.ts index 80180f073..0531aa520 100644 --- a/packages/macros/tests/glimmer/dependency-satisfies.test.ts +++ b/packages/macros/tests/glimmer/dependency-satisfies.test.ts @@ -30,11 +30,8 @@ describe('dependency satisfies', () => { }); test('in branch', () => { - let result = transform( - `{{#if (macroDependencySatisfies 'qunit' '^2.8.0')}}
{{else}}{{/if}}`, - { filename } - ); - expect(result).toEqual('
'); + let result = transform(`{{#if (macroDependencySatisfies 'qunit' '^2.8.0')}}red{{else}}blue{{/if}}`, { filename }); + expect(result).toEqual('red'); }); test('emits false for out-of-range package', () => { From 3fa43ed20699870bc8bffbcc6dfef011b93870d0 Mon Sep 17 00:00:00 2001 From: Mike Engel Date: Wed, 29 Nov 2023 23:16:33 +0100 Subject: [PATCH 3/4] fix: handle specific sub-expression cases --- packages/macros/src/glimmer/ast-transform.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/macros/src/glimmer/ast-transform.ts b/packages/macros/src/glimmer/ast-transform.ts index c0a29cea5..b206b617a 100644 --- a/packages/macros/src/glimmer/ast-transform.ts +++ b/packages/macros/src/glimmer/ast-transform.ts @@ -77,7 +77,7 @@ export function makeFirstTransform(opts: FirstTransformParams) { } }, }, - SubExpression(node: any) { + SubExpression(node: any, walker: { parent: { node: any } }) { if (node.path.type !== 'PathExpression') { return; } @@ -97,9 +97,18 @@ export function makeFirstTransform(opts: FirstTransformParams) { ); } if (node.path.original === 'macroDependencySatisfies') { - return env.syntax.builders.sexpr('macroCondition', [ - literal(dependencySatisfies(node, opts.packageRoot, moduleName, packageCache), env.syntax.builders), - ]); + console.log('parent', JSON.stringify(walker.parent, null, 2)); + console.log('node', JSON.stringify(node, null, 2)); + const staticValue = literal( + dependencySatisfies(node, opts.packageRoot, moduleName, packageCache), + env.syntax.builders + ); + // If this is a macro expression by itself, then turn it into a macroCondition for the second pass to prune. + // Otherwise assume it's being composed with another macro and evaluate it as a literal + if (walker.parent.node.path.original === 'if') { + return env.syntax.builders.sexpr('macroCondition', [staticValue]); + } + return staticValue; } }, MustacheStatement(node: any) { From 746124bee32205f731274e01889330b789a755c7 Mon Sep 17 00:00:00 2001 From: Mike Engel Date: Thu, 30 Nov 2023 09:56:04 +0100 Subject: [PATCH 4/4] fix: remove logs --- packages/macros/src/glimmer/ast-transform.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/macros/src/glimmer/ast-transform.ts b/packages/macros/src/glimmer/ast-transform.ts index b206b617a..8fb071e96 100644 --- a/packages/macros/src/glimmer/ast-transform.ts +++ b/packages/macros/src/glimmer/ast-transform.ts @@ -97,8 +97,6 @@ export function makeFirstTransform(opts: FirstTransformParams) { ); } if (node.path.original === 'macroDependencySatisfies') { - console.log('parent', JSON.stringify(walker.parent, null, 2)); - console.log('node', JSON.stringify(node, null, 2)); const staticValue = literal( dependencySatisfies(node, opts.packageRoot, moduleName, packageCache), env.syntax.builders