@attr has discrepencies with `attributeChangedCallback`

オープン
#117 コメント 4 件 リアクション 1 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
25/100
issue の種類
機能追加
明瞭さ
説明が足りない
活発さ
停滞
技術スタック
typescript
領域
frontend, web-dev

調査の方向性

まず、issue で説明されているネイティブな attributeChangedCallback の契約に照らして @attr の動作を追跡します。アプローチを選択する前に、提案された各解決策を確認し、望ましい callback 名、値、呼び出し頻度を明確にします。動作について合意し、現在の抽象化の不一致なしに選択したアプローチが実装されれば完了です。

索引モデルが issue の本文から書いたものです。

説明

@attr

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:

@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:

@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
  1. 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.
  1. 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.
  1. 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.
  2. 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
主要言語
TypeScript
スター
1.4k
フォーク
61
平均マージ
1日 15時間
マージ済み PR(30日)
8

環境構築

Codespaces で開く

このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

github/catalyst のほかの issue

github/catalyst の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。