aframevr/aframe-inspector

Selecting a-scene crashes the React UI on aframe 1.6.0 and master

vincentfretin opened this issue · 3 comments

image

It works on aframe 1.5.0

Context of the error:
props = {name: 'camera', schema: {…}, data: null, componentname: 'screenshot', isSingle: false, …}
props.entity.getDOMAttribute(props.componentname) is undefined

image

on master:

document.querySelector('a-scene').getDOMAttribute('screenshot')
undefined
document.querySelector('a-scene').getAttribute('screenshot')
{width: 4096, height: 2048, camera: null}

on aframe 1.5.0

document.querySelector('a-scene').getDOMAttribute('screenshot')
{}

It's an easy fix, I can just rewrite the line to

entity.getDOMAttribute(props.componentname)?.[props.name]

@mrxz Do you confirm the change of behavior of getDOMAttribute returning undefined instead of {} was expected for a component set withsceneEl.setAttribute('screenshot', '') https://github.com/aframevr/aframe/blob/c590f85b30e0577442e4bf73c3f5fdedf1f21eab/src/core/scene/a-scene.js#L83 here with the latest changes aframe changes?

mrxz commented

@mrxz Do you confirm the change of behavior of getDOMAttribute returning undefined instead of {} was expected for a component set withsceneEl.setAttribute('screenshot', '') https://github.com/aframevr/aframe/blob/c590f85b30e0577442e4bf73c3f5fdedf1f21eab/src/core/scene/a-scene.js#L83 here with the latest changes aframe changes?

@vincentfretin
Partially, one of the goals was to reduce memory usage, so the internal attrValue is only leased from the object pool when needed. As a result there are more situations where it will be undefined when accessed directly or through getDOMAttribute. In particular for object based single-property components and components with no/empty schemas.

I was, however, under the impression that the outward behaviour of getDOMAttribute hadn't changed for multi-property components. Mainly because this 8 year old unit test remained green, but I now see that the test has a bug as shallowDeepEqual(undefined, {}) surprisingly passes.

We should at least fix the test to change the tested result then?