Skip to content

Commit

Permalink
[BUGFIX] Ensure legacy path.parts matches existing semantics (#1583)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chancancode authored Mar 28, 2024
1 parent 4f73b20 commit d051884
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 5 deletions.
33 changes: 28 additions & 5 deletions packages/@glimmer/syntax/lib/v1/legacy-interop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>) {
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<string>) {
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('.');
},
});

Expand Down
51 changes: 51 additions & 0 deletions packages/@glimmer/syntax/test/legacy-interop-test.ts
Original file line number Diff line number Diff line change
@@ -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');
});

0 comments on commit d051884

Please sign in to comment.