Skip to content

Commit

Permalink
[BUGFIX] Ensure hash properties are reified lazily
Browse files Browse the repository at this point in the history
`(hash)` previously had two different types of behavior, one where it
was used as a reference and could get individual properties lazily, and
one where it was fully reified and exposed in JS. The lazy nature of
hash was very useful as it prevented having to realize the full cost of
each reference immediately, and it also meant that some values could
be accessed in conditionals and run assertions just in time. For
example, if someone was using a component that was loaded lazily via
an engine, you could render the component on a hash within a conditional
and it would not cause an error (because the component did not exist):

```hbs
<SomeComponent as |api|>
  <api.eagerComponent/>

  {{#if this.displayLazy}}
    <api.lazyComponent/>
  {{/if}}
</SomeComponent>
```

This PR restores this ability by having `(hash)` return a proxy rather
than an object, which allows us to access each individual value without
reifying the references immediately. For backwards compatibility, we
also allow users to set values on the proxy, so long as they are not
setting any of the values defined on the proxy initially, since those
are read-only references.
  • Loading branch information
Chris Garrett committed Apr 22, 2021
1 parent 5c4def2 commit 5ce55ac
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 4 deletions.
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/);
}

@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');
}
);

0 comments on commit 5ce55ac

Please sign in to comment.