Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] Ensure hash properties are reified lazily #1298

Merged
merged 1 commit into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions packages/@glimmer/integration-tests/test/helpers/hash-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,70 @@ class HashTest extends RenderTest {

this.assertHTML('Chad Hietala');
}

@test
'individual hash values are accessed lazily'(assert: Assert) {
class FooBar extends GlimmerishComponent {
firstName = 'Godfrey';

get lastName() {
assert.ok(false, 'lastName was accessed');

return;
}
}

this.registerComponent(
'Glimmer',
'FooBar',
`{{yield (hash firstName=@firstName lastName=this.lastName)}}`,
FooBar
);

this.render(`<FooBar @firstName="Godfrey" as |values|>{{values.firstName}}</FooBar>`);

this.assertHTML('Godfrey');
this.assertStableRerender();
}

@test
'defined hash keys cannot be updated'(assert: Assert) {
class FooBar extends GlimmerishComponent {
constructor(owner: object, args: { hash: Record<string, unknown> }) {
super(owner, args);
args.hash.firstName = 'Chad';
}
}

this.registerComponent('Glimmer', 'FooBar', `{{yield @hash}}`, FooBar);

assert.throws(() => {
this.render(`
<FooBar @hash={{hash firstName="Godfrey"}} as |values|>
{{values.firstName}} {{values.lastName}}
</FooBar>
`);
}, /You attempted to set the "firstName" value on an object generated using the \(hash\) helper|Assignment to read-only properties is not allowed in strict mode/);
}

@test
'undefined hash keys can be updated'() {
class FooBar extends GlimmerishComponent {
constructor(owner: object, args: { hash: Record<string, unknown> }) {
super(owner, args);
args.hash.lastName = 'Chan';
}
}

this.registerComponent('Glimmer', 'FooBar', `{{yield @hash}}`, FooBar);

this.render(
`<FooBar @hash={{hash firstName="Godfrey"}} as |values|>{{values.firstName}} {{values.lastName}}</FooBar>`
);

this.assertHTML('Godfrey Chan');
this.assertStableRerender();
}
}

jitSuite(HashTest);
96 changes: 92 additions & 4 deletions packages/@glimmer/runtime/lib/helpers/hash.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,96 @@
import { CapturedArguments, Dict } from '@glimmer/interfaces';
import { createComputeRef, Reference } from '@glimmer/reference';
import { reifyNamed } from '@glimmer/runtime';
import { DEBUG } from '@glimmer/env';
import { CapturedArguments, CapturedNamedArguments, Dict } from '@glimmer/interfaces';
import { setCustomTagFor } from '@glimmer/manager';
import { createConstRef, Reference, valueForRef } from '@glimmer/reference';
import { dict, HAS_NATIVE_PROXY } from '@glimmer/util';
import { Tag, track } from '@glimmer/validator';
import { assert } from '@glimmer/global-context';
import { internalHelper } from './internal-helper';

function tagForKey(hash: Record<string, unknown>, key: string): Tag {
return track(() => hash[key]);
}

let hashProxyFor: (args: CapturedNamedArguments) => Record<string, unknown>;

class HashProxy implements ProxyHandler<Record<string, unknown>> {
constructor(private named: CapturedNamedArguments) {}

get(target: Record<string, unknown>, prop: string | number, receiver: unknown) {
const ref = this.named[prop as string];

if (ref !== undefined) {
return valueForRef(ref);
} else {
return Reflect.get(target, prop, receiver);
}
}

set(target: Record<string, unknown>, prop: string | number, receiver: unknown) {
assert(
!(prop in this.named),
`You attempted to set the "${prop}" value on an object generated using the (hash) helper, but this is not supported because it was defined on the original hash and is a reference to the original value. You can set values which were _not_ defined on the hash, but you cannot set values defined on the original hash (e.g. {{hash myValue=123}})`
);

return Reflect.set(target, prop, receiver);
}

has(target: Record<string, unknown>, prop: string | number) {
return prop in this.named || prop in target;
}

ownKeys(target: {}) {
return Reflect.ownKeys(this.named).concat(Reflect.ownKeys(target));
}

getOwnPropertyDescriptor(target: {}, prop: string | number) {
if (DEBUG && !(prop in this.named)) {
throw new Error(
`args proxies do not have real property descriptors, so you should never need to call getOwnPropertyDescriptor yourself. This code exists for enumerability, such as in for-in loops and Object.keys(). Attempted to get the descriptor for \`${String(
prop
)}\``
);
}

if (prop in this.named) {
return {
enumerable: true,
configurable: true,
};
}

return Reflect.getOwnPropertyDescriptor(target, prop);
}
}

if (HAS_NATIVE_PROXY) {
hashProxyFor = (named) => {
const proxy = new Proxy(dict(), new HashProxy(named));

setCustomTagFor(proxy, (_obj: object, key: string) => tagForKey(named, key));

return proxy;
};
} else {
hashProxyFor = (named) => {
let proxy = dict();

Object.keys(named).forEach((name) => {
Object.defineProperty(proxy, name, {
enumerable: true,
configurable: true,
get() {
return valueForRef(named[name]);
},
});
});

setCustomTagFor(proxy, (_obj: object, key: string) => tagForKey(named, key));

return proxy;
};
}

/**
Use the `{{hash}}` helper to create a hash to pass as an option to your
components. This is specially useful for contextual components where you can
Expand Down Expand Up @@ -41,6 +129,6 @@ import { internalHelper } from './internal-helper';
*/
export default internalHelper(
({ named }: CapturedArguments): Reference<Dict<unknown>> => {
return createComputeRef(() => reifyNamed(named), null, 'hash');
return createConstRef(hashProxyFor(named), 'hash');
}
);