Passthrough of attributes on primitives such as a-sphere seems to be broken in 1.4.0

[diarmidmackenzie]

Description:

• A-Frame Version: 1.4.0
• Platform / Device: All
• Reproducible Code Snippet or URL: https://glitch.com/edit/#!/primitive-attribute-passthrough?path=index.html%3A32%3A7

Example shows a couple of different ways of setting the color & radius of an a-sphere.

      AFRAME.registerComponent('big-blue-1', {
        init() {
          this.el.setAttribute('geometry', 'radius: 0.75')
          this.el.setAttribute('material', 'color: blue')
        }
      })

      AFRAME.registerComponent('big-blue-2', {
        init() {
          this.el.setAttribute('radius', '0.75')
          this.el.setAttribute('color', 'blue')
        }
      })

In 1.3.0 both big-blue-1 and big-blue-2 work as they should.

In 1.4.0, big-blue-1 works OK, but big-blue-2 no longer alters the sphere's color or radius.

Found when updating aframe-physics-system examples to 1.4.0. I haven't yet looked at why this is. For now I am working around by updating my examples to the alternative approach that still works.

[dmarcos]

On https://aframe.glitch.me/ changing color works for me. bug gotta be more nuanced

[dmarcos]

What about if you set material and geometry as component dependencies?

[diarmidmackenzie]

Looks like some sort of race condition so only affects attributes set on entity creation. E.g. this works OK.

      AFRAME.registerComponent('big-blue-2', {
        init() {
          setTimeout(() => {
              this.el.setAttribute('radius', '0.75')
              this.el.setAttribute('color', 'blue')
          }, 0)
        }
      })

However, adding dependencies as you suggested doesn't fix things - this still doesn't work

      AFRAME.registerComponent('big-blue-2', {
       dependencies: ['geometry', 'material'],
       init() {
         this.el.setAttribute('radius', '0.75')
         this.el.setAttribute('color', 'blue')
       }
     })

Other observations:

• re-ordering the components in the <a-sphere> doesn't make any difference:

<a-sphere big-blue-2 position="1 1 -5" radius="0.5" color="#EF2D5E"></a-sphere>

• removing the clashing components in <a-sphere> doesn't make any difference either - this creates a white sphere of radius 1, rather than a blue sphere of radius 0.75:

<a-sphere big-blue-2 position="1 1 -5"></a-sphere>

[diarmidmackenzie]

By setting beakpoints, I've been able to obvserve that attributeChangedCallback() is not being called in the 1.4.0 case. I don't have time to investigate further just now...

[diarmidmackenzie]

I wrote a few additional unit tests to probe this area of function. These all work against 1.3.0. The last one fails against 1.4.0 / 1,4.1.

https://github.com/diarmidmackenzie/aframe/blob/4b535d8b2107bdc006a7238ecc0563ed5a3067c0/tests/extras/primitives/primitives/a-torus.test.js

The issue seems to be related to using an interim component to set the additional properties.

[diarmidmackenzie]

OK, this seems to be a fairly simple problem with, hopefully, a simple fix.

This issue is that the MutationObserver that's needed if attributeChangedCallback() is to be called is not set up until after the components for the entity are initialized.

So any work done in the init() of a component on the entity can't trigger updates of attributes that need to be passed through in this way.

Components are initialized in entityLoadCallback(), which is passed into load as a callback.

In the load() function in a-node.js, we execute the supplied callback before setting up the Mutaton Observers.

      if (cb) { cb(); }
      self.setupMutationObserver();
      

The fix may be as simple as swapping those two lines over. That fixes the UT I wrote. I'll create a PR for that and see whether all the UTs run cleanly over it.

[vincentfretin]

Really good investigation! We really didn't have tests that covered this case? Sad.
This is a regression from #5136
Previously we had a single MutationObserver that was registered really early on the first registerComponent call, so indeed before any component init, see https://github.com/dmarcos/document-register-element/blob/nativeSetAttributeBuild/src/document-register-element.js#L523
With #5136 changes we added this line self.setupMutationObserver() to register a MutationObserver for each node.

[diarmidmackenzie]

I know we released a 1.4.1 already, but I think it's worth considering a 1.4.2 with this fix in.

I had to fix up several examples in aframe-physics-system, and I would expect quite a few people will find this breaks their code when upgrading to 1.4.

@dmarcos?

[dmarcos]

Fixed by #5206