Skip to content

Misc Glimmer 2 fixes#13332

Merged
chancancode merged 7 commits intomasterfrom
glimmer-fixes
Apr 28, 2016
Merged

Misc Glimmer 2 fixes#13332
chancancode merged 7 commits intomasterfrom
glimmer-fixes

Conversation

@chancancode
Copy link
Member

@chancancode chancancode commented Apr 14, 2016

  • Glimmer compiler needs wire-format and references

  • Const reference optimization for dynamic component

    This optimizes {{component "foo-bar"}} into the same thing as {{foo-bar}} in the updating program. It also works for passed-in attrs like {{component @stuff}} where @stuff is static on the invocation side {{x-outside stuff="foo-bar"}}.

  • Correctly dirty Ember.set(foo, 'bar.baz.bat', ...)

    Previously, doing a Ember.set(foo, 'bar.baz.bat', ...) incorrectly marks both the "foo", "bar" and "baz" objects as dirty. This fixes the problem and only mark "bat" as dirty, i.e. making this the same as doing Ember.set(Ember.get(foo, 'bar.baz'), 'bat', ...).

  • Disable AST transforms on Glimmer for now

    This is causing issues because we have not implemented the mut helper in ember-glimmer (among other things). We can selectively re-enable stuff we still need (and works) as we go.

  • Make conditional helpers/references not volatile

    We previously marked them as volatile with a FIXME because we need to support proxies. This reduces the scope and only mark conditionals with proxies as volatile (we can still do better than that by making proxy objects have a special tag that combines its own DirtyableTag with the tag from its content, but it's pretty low priority at the moment).

    The refactor also added a new optimizations: by reusing the same code across the syntax ({{#if}}) and the helper ({{if}}) versions, the inline conditional helpers now also participate in the same const optimization, i.e. {{if true foo bar}} will be optimized into {{foo}} in the updating program, so as {{if @stuff foo bar}} if @stuff is static.

    TODO: fix tests that uses fake proxies, see comment in references.js (cc @chadhietala)

cc @krisselden

@wycats wycats force-pushed the glimmer-fixes branch 2 times, most recently from fe2f929 to c0f5a79 Compare April 14, 2016 20:12
registerPlugin('ast', TransformOldBindingSyntax);
registerPlugin('ast', TransformOldClassBindingSyntax);
registerPlugin('ast', TransformItemClass);
registerPlugin('ast', TransformClosureComponentAttrsIntoMut);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just guard this one? some of the others are needed to pass tests

@homu
Copy link
Contributor

homu commented Apr 16, 2016

☔ The latest upstream changes (presumably #13319) made this pull request unmergeable. Please resolve the merge conflicts.

Godhuda added 3 commits April 27, 2016 18:25
This makes `{{component "foo-bar"}}` more or less the same as `{{foo-bar}}`.
It also works for passed-in attrs like `{{component @stuff}}` where
`@stuff` is static on the invocation side `{{x-outside stuff="foo-bar"}}`.
Previously, doing a `Ember.set(foo, 'bar.baz.bat', ...)` incorrectly
marks both the "foo", "bar" and "baz" objects as dirty. This fixes the
problem and only mark "bat" as dirty, i.e. making this the same as doing
`Ember.set(Ember.get(foo, 'bar.baz'), 'bat', ...)`.
@wycats wycats force-pushed the glimmer-fixes branch 2 times, most recently from 42c0059 to 70ed331 Compare April 28, 2016 01:33
Godhuda added 4 commits April 27, 2016 19:03
Even if the source and path reference has not changed, the content could
still have changed, similar to how `PropertyReference` works.
This is causing issues because we have not implemented the `mut` helper
in `ember-glimmer` (among other things).
We previously marked them as `volatile` with a FIXME because we need to
support proxies. This reduces the scope and only mark conditionals with
proxies as volatile (we can still do better than that by making proxy
objects have a special tag that combines its own `DirtyableTag` with the
tag from its `content`, but it's pretty low priority at the moment).

The refactor also added a new optimizations: by reusing the same code
across the syntax (`{{#if}}`) and the helper (`{{if}}`) versions, the
inline conditional helpers now also participate in the same const
optimization, i.e. `{{if true foo bar}}` will be optimized into `{{foo}}`,
so as `{{if @stuff foo bar}}` if `@stuff` is static.

TODO: fix tests that uses fake proxies, see comment in `references.js`
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