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

Proper support for composable properties #96

Open
cibernox opened this issue Oct 29, 2014 · 3 comments
Open

Proper support for composable properties #96

cibernox opened this issue Oct 29, 2014 · 3 comments

Comments

@cibernox
Copy link
Owner

This is a long term plan for adding proper support to composable computed properties in a more efficient way.

There was a proposal that ended in a no-go by the core team. You can checkout this commit and search for Ember.FEATURES.isEnabled('composable-computed-properties') in the source code to see the original implementation.

The key difference with the current implementation is in the way computed properties that receive other computed properties subscribe to changes.

  • In this repo when a macro receives another macro as arguments, it inspects the dependencies of those computed properties and sets them as dependencies of the new macros. See https://github.com/cibernox/ember-cpm/blob/master/addon/utils.js#L38
  • The original approach is more efficient. Whenever an anonymous macros is created, it is registered in the global Ember namespace with a unique name (its guid). Therefore, when a macro receives a computed macros to compose the only thing it has to do is watch that key in the global Ember space.

This approach is more efficient since it sets up much less watches and also simplifies the code of the macros considerably. Also, should be more resilent to changes in ember itself, since it does not need to use private APIs of the computed properties.

The downside of this approach is that it can't be achieved completely from the plugins (I think), since it requires some changes in the guts of ember itself, by example in defineProperty (see this).

Therefore, to get this approach working a bit of bureaucracy will be necessary, either convince the core team to allow anonymous computed properties to be registered in the global namespace, or to provide a few extension points in key points of the core to allow to customize how thinks like defineProperty works.

Eventually the ultimate goal could be to offer a composable-compatible version to every macro built in ember to, eventually in ember 2.0, remove all computed properties from the core of ember (and therefore slim it of non superfluos) and let them live here.

/cc @truenorth @hjdivad

@mike-north
Copy link
Collaborator

Sounds like a good plan. AFAIK, what we have in place now is just about all that can be done without adding some missing support in the core library.

It would also be nice to leverage Ember.ReduceComputedProperty for macros like product instead of implementing the equivalent (w/ composeable CPM support) in utils.js

@hjdivad
Copy link
Collaborator

hjdivad commented Oct 29, 2014

👍

In my view the big win of this approach is that it makes it much easier for other users, and in particular users who may not be that familiar with ember internals, to write macros that can accept other macros as args.

If you look at the simple macros ember provides in the above commit, they're still fairly straightforward. That the macros in ember-cpm compose is awesome, but they're harder to write and reason about.

@kellyselden
Copy link
Contributor

Fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants