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

feat(ObserverLocator) observe computed properties via their dependencies #41

Merged
merged 1 commit into from
Feb 25, 2015
Merged

Conversation

jdanyow
Copy link
Contributor

@jdanyow jdanyow commented Feb 22, 2015

Fixes #15

@jdanyow
Copy link
Contributor Author

jdanyow commented Feb 22, 2015

Implemented this as a system-supplied adapter. To use in the skeleton, switch to aurelia-main and add the following:

main.js

import {ComputedObservationAdapter, ObjectObservationAdapter} from 'aurelia-framework';
...
...
aurelia.container.registerSingleton(ObjectObservationAdapter, ComputedObservationAdapter);

Then to avoid dirty checking Welcome.fullName, make the following changes:

welcome.js

import {declarePropertyDependencies} from 'aurelia-framework';

export class Welcome{
  ...
  ...
}

declarePropertyDependencies(Welcome, 'fullName', ['firstName', 'lastName']);

I'm guessing we'll take care of the main,js piece for people by adding something to the bootstrapper?

Other option was to hard code this thing in the ObserverLocator. I wasn't sure about doing that. Seems like there's more work to do in this area- need to come up with a design for handling properties with setters. Something similar to Object.getNotifier maybe.

@EisenbergEffect
Copy link
Contributor

I'm thinking we should just go ahead and "hard code" this one. What do you think? I don't think someone should have to turn this on. I think it's a reasonable default behavior.

FYI Once we get ES7 decorator support on Babel we will be able to do this:

export class Welcome {
  @dependsOn('firstName', 'lastName')
  fullName(){
    return this.firstName + ' ' + this.lastName;
  }
}

Thoughts on all this?

@jdanyow
Copy link
Contributor Author

jdanyow commented Feb 24, 2015

That's awesome- is this the best link for the ES7 decorators? I'm trying to figure out how closely the way we're currently "decorating" the property aligns with the future implementation. My guess is it's not close at all.

I agree the observe-via-dependencies option should be available by default- I'll refactor the pull request to discontinue using adapters and hard-code the condition in the ObserverLocator.

@EisenbergEffect
Copy link
Contributor

@jdanyow That's not the latest information on decorators. There's design work on that happening elsewhere. I'm not sure I should post the link here because I'm not sure if the repo author is prepared to handle community questions at this point. I'll put it in the private gitter. Suffice it to say, the design of decorators allows us to write code like above which can be used to attach metadata to the getter function.

@jdanyow
Copy link
Contributor Author

jdanyow commented Feb 25, 2015

made the changes- now the process would be this:

welcome.js

import {declarePropertyDependencies} from 'aurelia-framework';  // <-- add this

export class Welcome{
  ...
  ...
}

declarePropertyDependencies(Welcome, 'fullName', ['firstName', 'lastName']);  // <-- and this

ready to merge

@EisenbergEffect EisenbergEffect merged commit d2775f1 into aurelia:master Feb 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

observing computed properties via their dependencies
2 participants