aframe icon indicating copy to clipboard operation
aframe copied to clipboard

Implicit property type change from string to object from 1.5.0 to 1.6.0

Open Utopiah opened this issue 1 year ago • 3 comments

Description:

  • A-Frame Version: 1.5.0 to 1.6.0
  • Platform / Device: all
  • Reproducible Code Snippet or URL:

Before 1.6 I could do

AFRAME.registerComponent('componentname', {
  events: {
    released: function (evt) {
      console.log('This event was received!', evt);
      console.log( this.el.getAttribute('componentname') ); 
    }
  }
});

but now in 1.6 I must do

AFRAME.registerComponent('componentname', {
  schema: { type: 'string' },
  events: {
    released: function (evt) {
      console.log('This event was received!', evt);
      console.log( this.el.getAttribute('componentname') ); 
    }
  }
});

In 1.5 the output of e.g componentname="test" would be "test" but now it is {} unless the string type is forced via the schema.

Example https://glitch.com/edit/#!/aframe-16-implicit-component-type with output in the console.

This is not particularly a problem but it lead to unexpected behaviors for me. I briefly checked the change log and couldn't find it as a change. Is this expected? Documented?

Utopiah avatar Jun 14 '24 16:06 Utopiah

I think is unintended. In any case, I think it was undefined behavior. It makes probably sense to retain the string since it's what a regular HTML element would return. In A-Frame world the schema constrains the string into types and if there's not one we should keep the string @mrxz thoughts?

dmarcos avatar Jun 14 '24 17:06 dmarcos

This was indeed undefined behaviour prior to 1.6.0. Though even in 1.5.0 it doesn't do what you think it does. No schema and an empty schema ({}) both get interpreted the same way: as an object based schema without any properties. Giving it a string value is technically not valid, but can work <=1.5.0 under some conditions. What changed in 1.6.0 is that it prevents string values in all rather than some cases.

There are a couple of reasons this change was made:

  • It was always trying to parse the given string:
    el.setAttribute('onwireframeevent', 'Score: 100');
    console.log(el.getAttribute('onwireframeevent')); // { Score: "100" }
    
  • Once the data of the component became an object, any attempt to set it to a string afterwards would fail:
    el.setAttribute('onwireframeevent', ''); // Initialize the component
    // ...
    el.setAttribute('onwireframeevent', 'test');
    console.log(el.getAttribute('onwireframeevent')); // { 0: "t", 1: "e", 2: "s", 3: "t" }
    
  • It wasn't possible to (re)set it to an empty string
    el.setAttribute('onwireframeevent', 'test');
    el.setAttribute('onwireframeevent', '');
    console.log(el.getAttribute('onwireframeevent')); // "test"
    
  • Object based components lease from an object pool, setting a string value would cause leaks
  • It does not work with the inspector (as it also considers it an object-based component without properties)

In short, the 'new' behaviour is intended. While we could consider changing the semantics of no given schema, I'm not fond of the idea that no-schema and schema: {} would behave differently (unintuitive).

mrxz avatar Jun 15 '24 08:06 mrxz

no schema and schema : {} are both undefined to me should probably return a string

dmarcos avatar Jun 17 '24 14:06 dmarcos