-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BUGFIX release] Fix issue #14672 #14681
Conversation
f3d0a85
to
3e9b271
Compare
if (cache[keyName] !== CONSUMED) { | ||
cache[keyName] = CONSUMED; | ||
addDependentKeys(this, obj, keyName, meta); | ||
} | ||
return get(obj, this.altKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the above behavior is slightly different then how regular CP work: https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/computed.js#L338-L349
- we add the DK first, then invoke the
get
. Should we follow the pattern of regular CP, add the DK after the first get? - should
get
on alias be cached, until the DK changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change the order to be consistent, though getters really shouldn't have side effects. As for caching, it is likely cached on the other side, I'm just trying to track unconsumed vs not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
though getters really shouldn't have side effects.
They shouldn't but they often do, especially in development mode where we toString
objects, and users can provide there own toString
Not a big deal though, only doit if you are ok with it.
Trying to push an update to this but the push failed and but trying again says everything is up to date. |
PROPERTY_DID_CHANGE is the mechanism by which Ember's Glimmer refs are dirtied. Currently if something watches then unwatches an alias, the DK that is installed by the alias is removed.
bd9b61a
to
a6e5a82
Compare
@krisselden Will that come into the next 2.10 patch? |
@medokin yes |
@medokin released in 2.10.1 😄 |
PROPERTY_DID_CHANGE is the mechanism by which Ember's Glimmer refs are dirtied. Currently if something watches then unwatches an alias, the DK that is installed by the alias is removed.