Skip to content

Commit

Permalink
[BUGFIX] Fix {{get}} when used on non-objects
Browse files Browse the repository at this point in the history
Previously, `{{get}}` would not work on non-object values, even though
they may have had properties (e.g. `String.length`). This PR fixes this
and makes sure that `{{get}}` has the same behavior as directly
referencing the property in a template without using `{{get}}`.
  • Loading branch information
Chris Garrett committed Feb 12, 2021
1 parent ebd95ed commit 73da183
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
2 changes: 1 addition & 1 deletion packages/@glimmer/integration-tests/lib/modes/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ setGlobalContext({
let current: unknown = obj;

for (let part of parts) {
if (typeof current === 'function' || (typeof current === 'object' && current !== null)) {
if (current !== null && current !== undefined) {
current = (current as Record<string, unknown>)[part];
}
}
Expand Down
33 changes: 33 additions & 0 deletions packages/@glimmer/integration-tests/test/helpers/get-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,39 @@ class GetTest extends RenderTest {
this.rerender({ age: 30 });
this.assertHTML('miguelandrade30');
}

@test
'should be able to get string length with a static key'() {
this.render(`[{{get this.name 'length'}}] [{{if true (get this.name 'length')}}]`, {
name: 'Tomster',
});

this.assertHTML('[7] [7]');
this.assertStableRerender();

this.rerender({ name: 'Zoey' });
this.assertHTML('[4] [4]');

this.rerender({ name: 'Tomster' });
this.assertHTML('[7] [7]');
}

@test
'should be able to get string length with a bound/dynamic key'() {
this.render(`[{{get this.name this.key}}] [{{if true (get this.name this.key)}}]`, {
name: 'Tomster',
key: 'length',
});

this.assertHTML('[7] [7]');
this.assertStableRerender();

this.rerender({ key: 'foo' });
this.assertHTML('[] []');

this.rerender({ name: 'Zoey', key: 'length' });
this.assertHTML('[4] [4]');
}
}

jitSuite(GetTest);
10 changes: 4 additions & 6 deletions packages/@glimmer/runtime/lib/helpers/get.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getPath, setPath } from '@glimmer/global-context';
import { VMArguments } from '@glimmer/interfaces';
import { createComputeRef, valueForRef } from '@glimmer/reference';
import { isDict } from '@glimmer/util';
import { internalHelper } from './internal-helper';

/**
Expand Down Expand Up @@ -88,21 +89,18 @@ export default internalHelper((args: VMArguments) => {
() => {
let source = valueForRef(sourceRef);

if (isObject(source)) {
if (isDict(source)) {
debugger
return getPath(source, String(valueForRef(pathRef)));
}
},
(value) => {
let source = valueForRef(sourceRef);

if (isObject(source)) {
if (isDict(source)) {
return setPath(source, String(valueForRef(pathRef)), value);
}
},
'get'
);
});

function isObject(obj: unknown): obj is object {
return typeof obj === 'function' || (typeof obj === 'object' && obj !== null);
}

0 comments on commit 73da183

Please sign in to comment.