From d051884981c3f45f0833a03cddef707a38094da8 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Thu, 28 Mar 2024 13:42:09 -0700 Subject: [PATCH] [BUGFIX] Ensure legacy path.parts matches existing semantics (#1583) The refactor in #1568 slightly changed the semantics of `path.parts` in that it didn't previously include `this` or the leading `@`. This commit restores the previous semantics. --- .../@glimmer/syntax/lib/v1/legacy-interop.ts | 33 ++++++++++-- .../syntax/test/legacy-interop-test.ts | 51 +++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 packages/@glimmer/syntax/test/legacy-interop-test.ts diff --git a/packages/@glimmer/syntax/lib/v1/legacy-interop.ts b/packages/@glimmer/syntax/lib/v1/legacy-interop.ts index 404ba20d14..687a0787d3 100644 --- a/packages/@glimmer/syntax/lib/v1/legacy-interop.ts +++ b/packages/@glimmer/syntax/lib/v1/legacy-interop.ts @@ -61,12 +61,35 @@ export function buildLegacyPath({ head, tail, loc }: PathExpressionParams): ASTv Object.defineProperty(node, 'parts', { enumerable: false, get(this: { original: string }): readonly string[] { - deprecate(`The parts property on path nodes is deprecated, use trusting instead`); - return Object.freeze(this.original.split('.')); + deprecate(`The parts property on path nodes is deprecated, use head and tail instead`); + let parts = asPresentArray(this.original.split('.')); + + if (parts[0] === 'this') { + // parts does not include `this` + parts.shift(); + } else if (parts[0].startsWith('@')) { + // parts does not include leading `@` + parts[0] = parts[0].slice(1); + } + + return Object.freeze(parts); }, - set(this: { original: string }, value: PresentArray) { - deprecate(`The parts property on mustache nodes is deprecated, use trusting instead`); - this.original = value.join('.'); + set(this: { head: ASTv1.PathHead; original: string }, values: PresentArray) { + deprecate(`The parts property on mustache nodes is deprecated, use head and tail instead`); + + let parts = [...values]; + + // you are not supposed to already have `this` or `@` in the parts, but since this is + // deprecated anyway, we will infer what you meant and allow it + if (parts[0] !== 'this' && !parts[0]?.startsWith('@')) { + if (this.head.type === 'ThisHead') { + parts.unshift('this'); + } else if (this.head.type === 'AtHead') { + parts[0] = `@${parts[0]}`; + } + } + + this.original = parts.join('.'); }, }); diff --git a/packages/@glimmer/syntax/test/legacy-interop-test.ts b/packages/@glimmer/syntax/test/legacy-interop-test.ts new file mode 100644 index 0000000000..367f8a3948 --- /dev/null +++ b/packages/@glimmer/syntax/test/legacy-interop-test.ts @@ -0,0 +1,51 @@ +import { builders as b } from '@glimmer/syntax'; + +QUnit.module('[glimmer-syntax] AST nodes legacy interop'); + +QUnit.test('path.parts does not include this', (assert) => { + let path = b.path('this.foo.bar'); + + assert.deepEqual(path.original, 'this.foo.bar', 'path.original should include this'); + assert.deepEqual(path.head.type, 'ThisHead', 'path.head should be a ThisHead'); + assert.deepEqual(path.parts, ['foo', 'bar'], 'path.parts should not include this'); + + path.parts = ['bar', 'baz']; + + assert.deepEqual(path.original, 'this.bar.baz', 'path.original should include this'); + assert.deepEqual(path.head.type, 'ThisHead', 'path.head should be a ThisHead'); + assert.deepEqual(path.parts, ['bar', 'baz'], 'path.parts should not include this'); + + path.head = b.head('@foo'); + assert.deepEqual(path.head.type, 'AtHead', 'path.head should be a AtHead'); + + // Inconsistent, but we will allow it + path.parts = ['this', 'foo', 'bar', 'baz']; + + assert.deepEqual(path.original, 'this.foo.bar.baz', 'path.original should include this'); + assert.deepEqual(path.head.type, 'ThisHead', 'path.head should be a ThisHead'); + assert.deepEqual(path.parts, ['foo', 'bar', 'baz'], 'path.parts should not include this'); +}); + +QUnit.test('path.parts does not include @', (assert) => { + let path = b.path('@foo.bar'); + + assert.deepEqual(path.original, '@foo.bar', 'path.original should include @'); + assert.deepEqual(path.head.type, 'AtHead', 'path.head should be a AtHead'); + assert.deepEqual(path.parts, ['foo', 'bar'], 'path.parts should not include @'); + + path.parts = ['bar', 'baz']; + + assert.deepEqual(path.original, '@bar.baz', 'path.original should include @'); + assert.deepEqual(path.head.type, 'AtHead', 'path.head should be a AtHead'); + assert.deepEqual(path.parts, ['bar', 'baz'], 'path.parts should not include @'); + + path.head = b.head('this'); + assert.deepEqual(path.head.type, 'ThisHead', 'path.head should be a ThisHead'); + + // Inconsistent, but we will allow it + path.parts = ['@foo', 'bar', 'baz']; + + assert.deepEqual(path.original, '@foo.bar.baz', 'path.original should include @'); + assert.deepEqual(path.head.type, 'AtHead', 'path.head should be a AtHead'); + assert.deepEqual(path.parts, ['foo', 'bar', 'baz'], 'path.parts should not include this'); +});