Aylur / ags

A customizable and extensible shell
GNU General Public License v3.0
1.73k stars 94 forks source link

Service.updateProperty() seems to confuse prop names #440

Closed agriffis closed 4 weeks ago

agriffis commented 4 weeks ago

I'm looking at Service.updateProperty() and wondering if there may be a bug here.

https://github.com/Aylur/ags/blob/11150225e62462bcd431d1e55185e810190a730a/src/service.ts#L85-L98

This code expects to receive the kebab-cased property name, but the conditionalization checks this[prop] === value before converting to the private name.

In other words, it seems like it should be:

  updateProperty(prop: string, value: unknown) {
    const privateProp = ('_' +
      prop
        .split('-')
        .map((w, i) => (i > 0 ? w.charAt(0).toUpperCase() + w.slice(1) : w))
        .join('')) as keyof typeof this;

    if (
      this[privateProp] === value ||
      JSON.stringify(this[privateProp]) === JSON.stringify(value)
    )
      return;

    this[privateProp] = value as any;
    this.notify(prop);
  }
Aylur commented 4 weeks ago

both should work as expected gobject properties that are defined in registerClass and have a getter defined in snake_case will be mapped to kebab-case and camelCase too, which means there is no need to convert beforehand I had to look at the source because I wasn't sure why I assign to this[`_${privateProp}`], but thats just a convention that I used throughout services, for example the battery service.

agriffis commented 4 weeks ago

Got it, thanks!