github / github/catalyst

@attr has discrepencies with `attributeChangedCallback`

Aperta
#117 4 commenti 1 reazione 0 assegnatari Vedi su GitHub
@attr
Lingua principale
TypeScript
Stelle
1.4k
Fork
59
Merge medio
22h 12m
PR unite (30g)
5

Descrizione

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

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
frontend, web-dev
Tipo di issue
Funzionalità
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Ferma
Chiarezza
Da chiarire
Idoneità per principianti
25/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.