@attr has discrepencies with `attributeChangedCallback`
- Lenguaje dominante
- TypeScript
- Estrellas
- 1.4k
- Forks
- 59
- Merge medio
- 22 h 12 min
- PR fusionados (30 d)
- 5
Descripción
The `attributeChangedCallback` function is called whenever an observed attribute is set, added or removed. The `attributeChangedCallback` is called with the raw values of the attribute name, old value and new value.
This can be surprising when using the `@attr` decorator which gives conveniences over the attributes by allowing for camel-cased names eliding their `data-` prefix, and allowing types other than `string`. It can also be surprising to see `attributeChangedCallback` being called with `null` when you type an `@attr` as `string`.
For example:
```typescript
@controller
class FooBarElement extends HTMLElement {
@attr foo = true
attributeChangedCallback(name, oldValue, newValue) {
console.log(name, oldValue, newValue)
}
}
```
When this element first connects, `attributeChangedCallback('data-foo', null, '')` will fire. `attributeChangedCallback` will never fire with `'foo'` as the name, because that is a Catalyst concept and not a WC concept. The same is true of the `boolean` type; `attributeChangedCallback` will never fire with booleans for values. This means developers have to suddenly work around all of the conveniences `@attr` offers them:
```typescript
@controller
class FooBarElement extends HTMLElement {
@attr foo = true
attributeChangedCallback(name, oldValue, newValue) {
if (name === 'data-foo') { // Smell: `data-foo` is the name of the attribute but we call if `foo` everywhere else in the file
if (newValue === null) { // Smell: this should really be `if (oldValue === false)` to align with our expected types
doStuff();
}
}
}
}
```
### Possible Solutions
1. Document this gotcha and just let developers deal with it.
- This burdens developers with the extra concepts they have to keep in their brain
- It demonstrates that this is a leaky abstraction
- The code they eventually have to write still smells
2. Add extra code to call `attributeChangedCallback` with the mapped `name`, `oldValue`, `newValue`
- Potentially harmful: `attributeChangedCallback` has a fixed signature where it is only called with `string|null` and we're abusing that contract.
- This will cause the `attributeChangedCallback` to fire far more often, effectively double for each change to an `attr` mapped value. Use cases which do not care about the argument values will see more - effectively redundant - calls.
3. Add extra code to call a new function (maybe `attrChangedCallback`) with the mapped `name`, `oldValue`, `newValue`.
- This means further divergence from web component spec.
- More to document, more stuff that Catalyst is responsible for.
4. Add a function which can be given the 3 arguments and map it back to prop names (e.g. `const [realName, realOldValue, realNewValue] = this.attributeToAttr(name, oldValue, newValue)`)
- I hate it
- Still burdens developers with extra concepts
- Still demonstrates the leaky abstraction
- Still requires more documentation, more stuff that Catalyst is responsible for.
5. Add events which are automatically dispatched when attributes changed, which can be listened for. changing `HelloWorldElement`'s `@attr name = ''` value could emit `hello-world-name-changed`.
- It's still idiomatic to web components... sort of
- Elements become very noisy
- Required elements binding to their own events, so they're not in control of their own state.
- Still requires more documentation
Guía de contribución
Línea de trabajo
Start by tracing the @attr behavior against the native attributeChangedCallback contract described in the issue. Review each proposed solution and establish the desired callback names, values, and call frequency before choosing an approach. Done means the behavior is agreed and the selected approach is implemented without the current abstraction mismatch.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- typescript
- Área
- frontend, web-dev
- Tipo de issue
- Nueva funcionalidad
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Estancado
- Claridad
- Necesita aclaración
- Aptitud para principiantes
- 25/100