support runtime mixins (fixes #895) #1610
Conversation
| * For entities that already have been loaded by the time the mixin was attached, tell | ||
| * those entities to register the mixin and refresh their component data. | ||
| */ | ||
| refreshEntities: { |
There was a problem hiding this comment.
Can we use updateEntities to use a consistent language?
ngokevin
force-pushed
the
asyncmixin
branch
2 times, most recently
from
42512a8
to
2059255
Compare
| cacheAttribute: { | ||
| value: function (attr, value) { | ||
| var component = components[attr]; | ||
| if (!component) { return; } | ||
| value = value === undefined ? HTMLElement.prototype.getAttribute.call(this, attr) : value; | ||
| this.componentAttrCache[attr] = component.parseAttrValueForCache(value); | ||
| value = ( |
There was a problem hiding this comment.
What is this parenthesis? Can we collapse to one line?
There was a problem hiding this comment.
ok. any other comments for this PR?
| @@ -326,7 +326,7 @@ module.exports.AAnimation = registerElement('a-animation', { | |||
| * Works the same as component mixins but reimplemented because animations | |||
| * aren't components. | |||
| */ | |||
| applyMixin: { | |||
| handleMixinUpdate: { | |||
There was a problem hiding this comment.
apply mixin seems a better name than handleMixinUpdate. When invoked from the attachedCallback method there's no mixin update anywhere.
There was a problem hiding this comment.
Yeah, I guess so only because of <a-animation>. Otherwise for entities, it's purely a handler for the MutationObserver. I can rename it back for now.
Just to implant the idea in your head, I want to kill <a-animation> for a component-based approach. For timeline and such, I think we could find an API that meshes with ECS. Just bringing this up now because if we didn't have <a-animation>, we could transfer the mixin logic down to the Entity level, and ANode would purely be for load order/events.
There was a problem hiding this comment.
We can discuss about removing a-animation. I believe that should be solved by 3rd parties anyways.
Description:
Support mixins being attached/injected after entities have already loaded.
Changes proposed:
Component.updatePropertiesto take an undefinedattrValue. This means continue updating properties using mixins and such, but use cached attr value.applyMixintohandleMixinUpdateto signify that it is asynchronous and that it doesn't apply when the mixin is attached. It only handles observed mutations.componentAttrCachetocomponentCachebecause after parsing, anything related to the attribute value becomes irrelevant. It becomes purely component data.