Skip to content

Conversation

@chancancode
Copy link
Member

This commit restructures some of the more arcane parts of ember-metal's observer system.

It eliminates some special casing around length, which isn't necessary anymore but complicates the overall abstraction of observable properties.

Relatedly, it also finishes implementing PROPERTY_DID_CHANGE, which now executes reliably any time an observer would have fired. You can think of it as an always-on observer if it's present on an object. Glimmer 2 curly components use this hook to propagate sets on components back to their original bindings (if the bindings are present).

This commit also implements two way bindings in Glimmer 2, by making PropertyReferences implement an UPDATE method. If that method is present, it is used to propagate changes upwards (otherwise, an exception is thrown).

@wycats wycats force-pushed the two-way-bindings branch 3 times, most recently from ae3e4b0 to 715b38d Compare May 16, 2016 00:59
@chancancode
Copy link
Member Author

Follow-up TODO: Implement mut as an internal helper for backwards compatibility. The Glimmer 2 system doesn't "need" it, but the mut helper has observable effects (putting mutable cells in attrs) that we might still want/need to support.

The mut helper probably just needs to return a wrapped reference with a special symbol on it, and process-args.js can do the right thing accordingly.

Similarly, we also need readonly as an internal helper which wraps the reference and throws in [UPDATE].

@Serabe
Copy link
Member

Serabe commented May 16, 2016

Twiddle

The twiddle shows three different invocations:

{{my-component foo=1 bar=(inc 0)}}
{{my-component foo=foo bar=(inc bar)}}
{{my-component foo=(readonly foo) bar=(readonly (inc bar))}}

Notice that there are no {{my-component foo=(mut foo) bar=(inc bar)}} given that the second option already covers this (path expressions in arguments for components get a @mut automatically in compilation time).

my-component shows {{foo}} - {{bar}} with two buttons for changing the properties foo and bar in the component. Furthermore, there are two buttons to reset foo and bar in the application controller.

Clicking the increment buttons for the first invocation does not raise an error nor produce strange behaviour.

Incrementing foo in the second invocation will set foo in the third invocation to the same value (reset both external value and then increment the third foo three times. When incrementing the second foo once, the third foo will have the value 2 instead of 4 since the binding is shared).

Incrementing foo in the third invocation will have no side effects.

Incrementing bar in any invocation will only change the inner value, given that bar is the output of a helper.

Funny thing, incrementing the value of foo in the second invocation will reset bar in both the second and the third invocation (this is actually expected since a rerender is caused and it is getting the result value from (inc bar) again and that is always 1).

var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;

if (!watching) {
if (m && !m.isInitialized(obj)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much more clear

@krisselden
Copy link
Contributor

Overall this looks good to me.

This commit restructures some of the more arcane parts of ember-metal's
observer system.

It eliminates some special casing around `length`, which isn't necessary
anymore but complicates the overall abstraction of observable
properties.

Relatedly, it also finishes implementing PROPERTY_DID_CHANGE, which now
executes reliably any time an observer would have fired. You can think
of it as an always-on observer if it's present on an object. Glimmer 2
curly components use this hook to propagate `set`s on components back to
their original bindings (if the bindings are present).

This commit also implements two way bindings in Glimmer 2, by making
PropertyReferences implement an `UPDATE` method. If that method is
present, it is used to propagate changes upwards (otherwise, an
exception is thrown).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants