Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Utopiah opened this issue Jun 14, 2024 · 3 comments
Open

Comments

@Utopiah
Copy link
Contributor

Utopiah commented Jun 14, 2024

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?

@dmarcos
Copy link
Member

dmarcos commented Jun 14, 2024

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
Copy link
Contributor

mrxz commented Jun 15, 2024

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

@dmarcos
Copy link
Member

dmarcos commented Jun 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants