aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.69k stars 3.98k forks source link

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

Open Utopiah opened 5 months ago

Utopiah commented 5 months ago

Description:

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?

dmarcos commented 5 months ago

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?

mrxz commented 5 months ago

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:

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).

dmarcos commented 5 months ago

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