From 183dd97a093a81369df607facbde0e6bc6135e2f Mon Sep 17 00:00:00 2001 From: Simon Ihmig Date: Fri, 23 Feb 2024 17:24:46 +0100 Subject: [PATCH 1/2] Make macro configs (except fastboot) be considered not having a runtime implementation --- packages/macros/src/babel/evaluate-json.ts | 47 +++++++++++++++++-- packages/macros/tests/babel/eval.test.ts | 40 +++++++++++++++- .../tests/babel/macro-condition.test.ts | 15 +----- 3 files changed, 83 insertions(+), 19 deletions(-) diff --git a/packages/macros/src/babel/evaluate-json.ts b/packages/macros/src/babel/evaluate-json.ts index 135f56d0c..65bade79c 100644 --- a/packages/macros/src/babel/evaluate-json.ts +++ b/packages/macros/src/babel/evaluate-json.ts @@ -5,6 +5,7 @@ import type State from './state'; import dependencySatisfies from './dependency-satisfies'; import moduleExists from './module-exists'; import getConfig from './get-config'; +import assertNever from 'assert-never'; type OpValue = string | boolean | number; @@ -388,13 +389,53 @@ export class Evaluator { return { confident: true, value: moduleExists(path, this.state), hasRuntimeImplementation: false }; } if (callee.referencesImport('@embroider/macros', 'getConfig')) { - return { confident: true, value: getConfig(path, this.state, 'package'), hasRuntimeImplementation: true }; + return { confident: true, value: getConfig(path, this.state, 'package'), hasRuntimeImplementation: false }; } if (callee.referencesImport('@embroider/macros', 'getOwnConfig')) { - return { confident: true, value: getConfig(path, this.state, 'own'), hasRuntimeImplementation: true }; + return { confident: true, value: getConfig(path, this.state, 'own'), hasRuntimeImplementation: false }; } if (callee.referencesImport('@embroider/macros', 'getGlobalConfig')) { - return { confident: true, value: getConfig(path, this.state, 'getGlobalConfig'), hasRuntimeImplementation: true }; + // Check for getGlobalConfig().fastboot.isRunning, which is the only pattern in use where config actually needs to have a runtime implementation. + // For compatibility reasons we will continue to support that. All other cases of macro configs are static now. + let maybeFastbootMemberExpression = path.parentPath; + if ( + maybeFastbootMemberExpression.isMemberExpression() || + maybeFastbootMemberExpression.isOptionalMemberExpression() + ) { + let maybeFastbootProperty = maybeFastbootMemberExpression.isMemberExpression() + ? maybeFastbootMemberExpression.get('property') + : maybeFastbootMemberExpression.isOptionalMemberExpression() + ? maybeFastbootMemberExpression.get('property') + : assertNever(maybeFastbootMemberExpression); + + if (maybeFastbootProperty.isIdentifier() && maybeFastbootProperty.node.name === 'fastboot') { + let maybeIsRunningMemberExpression = maybeFastbootMemberExpression.parentPath; + if ( + maybeIsRunningMemberExpression.isMemberExpression() || + maybeIsRunningMemberExpression.isOptionalMemberExpression() + ) { + let maybeIsRunningProperty = maybeIsRunningMemberExpression.isMemberExpression() + ? maybeIsRunningMemberExpression.get('property') + : maybeIsRunningMemberExpression.isOptionalMemberExpression() + ? maybeIsRunningMemberExpression.get('property') + : assertNever(maybeIsRunningMemberExpression); + + if (maybeIsRunningProperty.isIdentifier() && maybeIsRunningProperty.node.name === 'isRunning') { + return { + confident: true, + value: getConfig(path, this.state, 'getGlobalConfig'), + hasRuntimeImplementation: true, + }; + } + } + } + } + + return { + confident: true, + value: getConfig(path, this.state, 'getGlobalConfig'), + hasRuntimeImplementation: false, + }; } if (callee.referencesImport('@embroider/macros', 'isDevelopingApp')) { return { diff --git a/packages/macros/tests/babel/eval.test.ts b/packages/macros/tests/babel/eval.test.ts index 2ef0468c2..bae0f81a3 100644 --- a/packages/macros/tests/babel/eval.test.ts +++ b/packages/macros/tests/babel/eval.test.ts @@ -164,7 +164,14 @@ describe('hasRuntimeImplementation', function () { let code = transform(` import { getConfig } from '@embroider/macros'; const result = getConfig('foo')`); - expect(code).toMatch(`result = true`); + expect(code).toMatch(`result = false`); + }); + + test('getConfig property access', () => { + let code = transform(` + import { getConfig } from '@embroider/macros'; + const result = getConfig('foo').bar`); + expect(code).toMatch(`result = false`); }); // this is throwing internally, not sure how to fix @@ -172,13 +179,42 @@ describe('hasRuntimeImplementation', function () { let code = transform(` import { getOwnConfig } from '@embroider/macros'; const result = getOwnConfig()`); - expect(code).toMatch(`result = true`); + expect(code).toMatch(`result = false`); }); test('getGlobalConfig', () => { let code = transform(` import { getGlobalConfig } from '@embroider/macros'; const result = getGlobalConfig()`); + expect(code).toMatch(`result = false`); + }); + + test('getGlobalConfig property access', () => { + let code = transform(` + import { getGlobalConfig } from '@embroider/macros'; + const result = getGlobalConfig().foo`); + expect(code).toMatch(`result = false`); + }); + + test('getGlobalConfig fastboot access', () => { + let code = transform(` + import { getGlobalConfig } from '@embroider/macros'; + const result = getGlobalConfig().fastboot`); + expect(code).toMatch(`result = false`); + }); + + // fastboot.isRunning relies on dynamic evaluation at runtime. For backwards compatibility we keep it working. See https://github.com/embroider-build/embroider/issues/1804 + test('getGlobalConfig fastboot.isRunning access', () => { + let code = transform(` + import { getGlobalConfig } from '@embroider/macros'; + const result = getGlobalConfig().fastboot.isRunning`); + expect(code).toMatch(`result = true`); + }); + + test('getGlobalConfig fastboot?.isRunning access', () => { + let code = transform(` + import { getGlobalConfig } from '@embroider/macros'; + const result = getGlobalConfig().fastboot?.isRunning`); expect(code).toMatch(`result = true`); }); diff --git a/packages/macros/tests/babel/macro-condition.test.ts b/packages/macros/tests/babel/macro-condition.test.ts index 723b14035..09a86de3a 100644 --- a/packages/macros/tests/babel/macro-condition.test.ts +++ b/packages/macros/tests/babel/macro-condition.test.ts @@ -360,7 +360,7 @@ describe('macroCondition', function () { expect(code).not.toMatch(/beta/); }); - buildTimeTest('can see booleans inside getConfig', () => { + test('can see booleans inside getConfig', () => { let code = transform(` import { macroCondition, getConfig } from '@embroider/macros'; export default function() { @@ -373,19 +373,6 @@ describe('macroCondition', function () { expect(code).not.toMatch(/alpha/); }); - runTimeTest('can see booleans inside getConfig', () => { - let code = transform(` - import { macroCondition, getConfig } from '@embroider/macros'; - export default function() { - // this deliberately chains three kinds of property access syntax: by - // identifier, by numeric index, and by string literal. - return macroCondition(getConfig('qunit').items[0]["other"]) ? 'alpha' : 'beta'; - } - `); - expect(run(code, { filename })).toBe('beta'); - expect(code).toMatch(/alpha/); - }); - if (transform.babelMajorVersion === 7) { buildTimeTest('can be used as class field initializer', () => { let code = transform(` From e55b05521885772abb056351744a607c397e17b5 Mon Sep 17 00:00:00 2001 From: Simon Ihmig Date: Tue, 27 Feb 2024 18:00:44 +0100 Subject: [PATCH 2/2] Treat getGlobalConfig().fastboot as dynamic as well --- packages/macros/src/babel/evaluate-json.ts | 24 +++++----------------- packages/macros/tests/babel/eval.test.ts | 2 +- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/macros/src/babel/evaluate-json.ts b/packages/macros/src/babel/evaluate-json.ts index 65bade79c..0782097ec 100644 --- a/packages/macros/src/babel/evaluate-json.ts +++ b/packages/macros/src/babel/evaluate-json.ts @@ -409,25 +409,11 @@ export class Evaluator { : assertNever(maybeFastbootMemberExpression); if (maybeFastbootProperty.isIdentifier() && maybeFastbootProperty.node.name === 'fastboot') { - let maybeIsRunningMemberExpression = maybeFastbootMemberExpression.parentPath; - if ( - maybeIsRunningMemberExpression.isMemberExpression() || - maybeIsRunningMemberExpression.isOptionalMemberExpression() - ) { - let maybeIsRunningProperty = maybeIsRunningMemberExpression.isMemberExpression() - ? maybeIsRunningMemberExpression.get('property') - : maybeIsRunningMemberExpression.isOptionalMemberExpression() - ? maybeIsRunningMemberExpression.get('property') - : assertNever(maybeIsRunningMemberExpression); - - if (maybeIsRunningProperty.isIdentifier() && maybeIsRunningProperty.node.name === 'isRunning') { - return { - confident: true, - value: getConfig(path, this.state, 'getGlobalConfig'), - hasRuntimeImplementation: true, - }; - } - } + return { + confident: true, + value: getConfig(path, this.state, 'getGlobalConfig'), + hasRuntimeImplementation: true, + }; } } diff --git a/packages/macros/tests/babel/eval.test.ts b/packages/macros/tests/babel/eval.test.ts index bae0f81a3..21d35a7ae 100644 --- a/packages/macros/tests/babel/eval.test.ts +++ b/packages/macros/tests/babel/eval.test.ts @@ -200,7 +200,7 @@ describe('hasRuntimeImplementation', function () { let code = transform(` import { getGlobalConfig } from '@embroider/macros'; const result = getGlobalConfig().fastboot`); - expect(code).toMatch(`result = false`); + expect(code).toMatch(`result = true`); }); // fastboot.isRunning relies on dynamic evaluation at runtime. For backwards compatibility we keep it working. See https://github.com/embroider-build/embroider/issues/1804