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

Tracked Properties Updates #478

Merged
merged 5 commits into from
May 6, 2019
Merged

Tracked Properties Updates #478

merged 5 commits into from
May 6, 2019

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Apr 18, 2019

@jonnii
Copy link

jonnii commented Apr 18, 2019

It's arguable that computed properties that are relying on these caching semantics are problematic in general. After all, it's strange to setup state like this during construction, usually you would use a CP instead, and if CPs are trying to use a value, it generally means that value should be a dependency.

I understand what's happening here but my issue with the fix is it seems like it requires you to put the @watchable attribute on the class which is doing the right thing. If the behavior is fixed there would be no feedback that this attribute is now no longer required. Perhaps using @watchable should also come with a deprecation warning or a linting hint to encourage changing the flow to be a cp/dependency and/or migrate to native classes?

@pzuraq
Copy link
Contributor Author

pzuraq commented Apr 18, 2019

We tend to do deprecations as separate RFCs from introducing new features. Computed properties aren't officially deprecated yet, though they likely will be sometime in the future. Until then, I don't think we should provide a deprecation warning for users of computed.

A linting rule may make more sense here. There is precedent with the "avoid-observers" eslint rules, for instance. I don't believe we need an RFC to create a rule like that though.

@rwjblue
Copy link
Member

rwjblue commented Apr 18, 2019

I don't believe we need an RFC to create a rule like that though

Agree, we can create the rule without any RFC type things. We'd only need to RFC enabling it by default.

@MelSumner MelSumner added Final Comment Period T-framework RFCs that impact the ember.js library labels Apr 20, 2019
@MelSumner
Copy link
Contributor

Since this is important to the interop story, we've decided to move this into FCP.

@luxzeitlos
Copy link

Basically a @tracked is always @watchable, right? Why would I then ever need to use @watchable and not @tracked?
From my perspective @tracked does no harm if used too much, right?

@pzuraq
Copy link
Contributor Author

pzuraq commented Apr 23, 2019

@tracked cannot and should not be used on native getters and setters. In the long run, all getters and setters should be native, without any decoration.

@watchable can only be used on native getters and setters, by contrast. It is only for compatibility, and eventually should be removed.

If we made @tracked work on both, it would quickly get confusing - users might think they need to mark everything as @tracked, which they do not, and they will find it much harder to quickly search for code that should be updated and refactored.

However, there still are two cases where you _will_ need to use them:

- When accessing and updating plain, undecorated properties on objects
- When using Ember's `ObjectProxy` class
Copy link
Member

Choose a reason for hiding this comment

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

Missing a third case: manual proxies that implement unknownProperty and setUnknownProperty

@mixonic
Copy link
Member

mixonic commented Apr 26, 2019

As we've discussed a @classic decorator in RFC 468 and this is also viewed as a somewhat temporary compatibility/migration path item, I'll float @classicDependency

@rwjblue rwjblue merged commit a47c955 into master May 6, 2019
@rwjblue rwjblue deleted the tracked-properties-updates branch May 6, 2019 13:00
rwjblue added a commit to rwjblue/ember-rfc176-data that referenced this pull request Aug 28, 2019
rwjblue added a commit to rwjblue/ember-cli-babel that referenced this pull request Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period T-framework RFCs that impact the ember.js library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants