github / github/catalyst

@attr has discrepencies with `attributeChangedCallback`

Abierto
#117 4 comentarios 1 reacción 0 asignados Ver en GitHub
@attr
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

Abrir la 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

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.