Silent failure where components not initialized if they are modified immediately after object creation.

[diarmidmackenzie]

Description:

• A-Frame Version: 1.2.0
• Platform / Device: Chrome browser
• Reproducible Code Snippet or URL: https://glitch.com/edit/#!/modifying-attributes-after-creation?path=index.html%3A34%3A7

A problem that I have hit, which may be due to A-Frame restrictions around entity creation, but it would be good to at least get a clear error message when making unsupported API calls, rather than (as currently happens) getting a silent failure to initialize the component.

I have a very simple bit of code that is creating an entity and applying a component to it. However, after appending the entity to the scene (before it has initialized), it may further modify the attributes on the entity, as per the example below.

(this example is simplified: in my case the last 2 lines of code occur in a different context, where it's less obvious the component may still be initializing).

When it does this, no error messages are generated, but the component initialization never actually happens at all.

          const box = document.createElement('a-box');
          box.setAttribute('make-red', '');
          this.el.appendChild(box);
          box.removeAttribute('make-red');
          box.setAttribute('make-red', '');

It would be ideal if these further attributes updates could be processed and applied when the component initializes. However I recognize this may not be possible.

I note the docs here do say that operations after appendChild() and before initialization completes, are limited:
https://aframe.io/docs/1.2.0/introduction/javascript-events-dom-apis.html#adding-an-entity-with-appendchild

"we can’t do many operations on the entity (such as calling .getAttribute()"

Assuming that setAttribute() is forbidden as well as getAttribute(), it would be great to do a couple of things:

• Could we generate an explicit error message on this setAttribute() call, rather than just silently failing? That would have helped me to track down the problem a lot faster.
• Can we update the docs to clarify that setAttribute() is disallowed, as well as getAttribute().

I'd be happy to work on a PR for the above, if you agree it would be a worthwhile change to make.

[arpu]

For me this looks more a problem in the box component not use the update() https://github.com/aframevr/aframe/blob/master/src/geometries/box.js

[diarmidmackenzie]

@arpu, I'm not sure I understand. There is no box component in the example.

There is a box geometry (which you linked to - this doesn't need an update() method) and the a-box primitive.

I'm pretty sure this same problem would reproduce with an just the same.

[diarmidmackenzie]

Looking into why this does fail, it looks like the issue is that, in the case above

• setAttribute is performed synchronously
• removeAttribute happens asynchronouslyy, only on the "componentinitialized" event.

So the issue is that the removeAttribute ends up getting acted on after the setAttribute operation, and hence the component ends up getting removed.

A potential fix would be for setAttribute to check for any pending tryRemoveLater event listener on the component, and remove any that is found.

The event listener is set up here:

FIx would be made somewhere around here:

My understanding is that there is no way to directly check for attached event listeners, so we'd need some additional state to track the pending removal.

[diarmidmackenzie]

Or a simple (more self-contained fix) might be here:

aframe/src/core/a-entity.js

Line 489 in 269d178

// Component already initialized. Update component.

// Component already initialized. Update component.
        component.updateProperties(attrValue, clobber);

to something like this (not tested yet):

if (component.initialized) {
         // Component already initialized. Update component.
         component.updateProperties(attrValue, clobber);
       }
       else {
         // Component initialization pending.  queue up this update.
         this.addEventListener('componentinitialized', function tryUpdateLater (evt) {
           if (evt.detail.name !== attr) { return; }
           component.updateProperties(attrValue, clobber);            
         });
       }

[diarmidmackenzie]

Fix proved to be a little more complicated

• After some experimenting I went with my original idea of adding an additional state flag to track this "pending removal" state of a component.
• Complicated by the fact that the DOM attribute gets deleted synchronously on the removeAttribute(), but doesn't get added back in on the subsequent setAttribute(), because the component is already in the process of initializing. I solved this by adding it back in on the basis of the "pending removal" flag being set.

I'll offer my fix in a PR.