Fix shouldUpdate in entity-mixin-lit #482
Conversation
PR Checklist:Did you use a semver keyword in a commit message?
Did you paste the Rally US URL in the description?
|
| @@ -35,7 +35,7 @@ export const EntityMixinLit = superclass => class extends superclass { | |||
| this.href && this.token) { | |||
| this._getEntity(); | |||
| } | |||
| return this.href && this.token; | |||
| return this.href && this.token ? super.shouldUpdate(changedProperties) : false; | |||
There was a problem hiding this comment.
Actually... localize-mixin.js had to do a bunch of extra work to accomplish something similar, so I'm worried something is getting lost here. Like it's remembering the list of updated properties when it returns false so that later when it can call super.shouldUpdate(changedProperties) it can include all the ones that it skipped.
There was a problem hiding this comment.
Oh interesting, I can look into that
There was a problem hiding this comment.
It's possible that it's overkill... you could probably just remember if super.shouldUpdate(changedProperties) would have ever returned true (when you're returning false because href/token aren't defined yet) and then if so, return that value.
There was a problem hiding this comment.
Yeah interesting, I think it's overkill for the use cases I know about because the localizeMixin doesn't actually use the changedProperties. But there could be another mixin after it that does... could this be turned into a controller mixins could use? 🤔
I'm a bit worried about this change, but in theory it should only be a good thing. Basically, anything using both the
EntityMixinLitandcore'sLocalizeDynamicMixinisn't actually waiting until the langterms are loaded before rendering. This is probably only actually noticeable in the OSLO case, where langterms take a while to return.I think the fix below is the correct way to do this -
EntityMixinLitgets the chance to say "no, don't update, we don't have anhref/token" but will then default to other mixin's requirements if its good to go with the update.Thoughts?