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] Updates @tracked to match the Tracked Properties RFC #17572

Merged
merged 4 commits into from
Feb 9, 2019

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Feb 6, 2019

Updates @tracked to be a classic decorator as well as a standard
native decorator, removes the ability to decorator native getters, and
adds more tests for various situations and updates the existing tests.

One thing to note is that users can still provide getters/setters to
classic classes via tracked:

import EmberObject from '@ember/object';
import { tracked } from '@glimmer/tracking';

const Person = EmberObject.extend({
  firstName: tracked({ value: 'Tom' }),
  lastName: tracked({ value: 'Dale' }),

  fullName: tracked({
    get() {
      return `${this.firstName} ${this.lastName}`;
    },
  }),
});

The reasoning behind this is there is no way to do this currently, and
it allows us to match behavior between native/classic exactly.
Alternatively we could expose the native descriptor decorator that is
internal only right now, but unfortunately computed will not be able
to fill this role since it requires use of Ember.set, where native
getters/setters do not.

TODO:

@pzuraq pzuraq changed the title [WIP][FEAT] Updates @Tracked to match the Tracked Properties RFC [WIP][FEAT] Updates @tracked to match the Tracked Properties RFC Feb 6, 2019
@pzuraq pzuraq changed the title [WIP][FEAT] Updates @tracked to match the Tracked Properties RFC [FEAT] Updates @tracked to match the Tracked Properties RFC Feb 7, 2019
@pzuraq pzuraq force-pushed the update-tracked branch 3 times, most recently from c3e98c2 to 99cb377 Compare February 8, 2019 01:40
@rwjblue
Copy link
Member

rwjblue commented Feb 8, 2019

@pzuraq - Can you fix the package.json conflict here?

Updates @Tracked to be a classic decorator as well as a standard
native decorator, removes the ability to decorator native getters, and
adds more tests for various situations and updates the existing tests.

This PR also exposes the native `descriptor` decorator for use defining
getters and setters:

```js
import EmberObject, { descriptor } from '@ember/object';
import { tracked } from '@glimmer/tracking';

const Person = EmberObject.extend({
  firstName: tracked({ value: 'Tom' }),
  lastName: tracked({ value: 'Dale' }),

  fullName: descriptor({
    get() {
      return `${this.firstName} ${this.lastName}`;
    },
  }),
});
```
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

I think we may be missing some tests around recomputing during rendering.

@pzuraq - Would you mind adding some tests that use getters that then consume tracked properties in a template?

import { setComputedDecorator } from './lib/decorator';
import { tracked } from './lib/tracked';

// We have to set this here because there is a cycle of dependencies in tracked
Copy link
Member

Choose a reason for hiding this comment

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

We should work to remove this cycle before landing

@krisselden krisselden merged commit c17949f into emberjs:master Feb 9, 2019
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

Successfully merging this pull request may close these issues.

3 participants