-
Notifications
You must be signed in to change notification settings - Fork 46
Bind to content #11
base: master
Are you sure you want to change the base?
Bind to content #11
Conversation
なるほどー bind は気持ち的にはクリックしたことだけうけとって、さくっと chrome 側の処理を追加したい! みたいな感じでつくったのでそうなってしまってました。(content 用のイベントは post() 内で普通に addEventListener() してしまう気持ち) bindToContent 自体はいれてしまおうと思います! ありがとうございます! |
あ、やはりそうでしたか。
|
ちょっと話がずれるのですが、bind()の具体的なユースケースというのはどういうものが想定されているんでしょうか? 今のbind()はメソッド名や引数に取れる内容、あと、用例でもコールバック関数がイベントらしきオブジェクトを受け取っているように見えることから、汎用的に使える事を期待させてしまうインターフェースになってしまっているので、自分のような勘違いをしてしまう人が出てくるんじゃないか……という気がしています。 bindToContent()はネーミング的にJSDeferredの既存の文法(メソッド名は基本的に1語で、存在をあまり意識させない)に則っていないので、何かいい名前が欲しいですね。 |
確かにそうですね > onClickOn みたいな |
post() はコンテントの中で実行されている感が多少名前からわかりますが、単に click() とかだと関数が chrome で実行されるのか content で実行されるのかわからないですね…… widget に追加するメソッドは全て content で実行されるように統一して、next() で繋げると chrome 側に処理が渡るとかでしょうか…… 通常の Deferred だと next() に入れた関数が複数回実行されることを想定しないのでこれはこれで気持ち悪いですが……
あるいは中間オブジェクトをはさんで jQuery っぽくしつつ、さらに関数名でコンテキストを明示するという気持ち悪い方法とか…… (これだと click() とかの返り値を Deferred にできない) 関数名で役割を明示する方法は既に Deferred.chain() というので存在してるのでそんなに悪いとは思ってないんですが、普通に考えるとキモい。
|
うおお……これは確かにJSDeferredらしくないですね。 |
ぼんやり考えてたのですが、Deferredそのものが持つネイティブな機能と、WidgetやPanelに追加される便利機能とは、色々分けて考えた方がいいのかなと思いました。
のような感じであらかじめ機能を拡張した物を提供する(そこまでがJSDeferredのJetpackバインディングの機能と考える)とか。 |
今のJSDeferredをベースにすると、実装としてはこんな感じでしょうか……
|
リッチなのはリッチなのであってもいいですが最低限のだけつけときたいですね (bind() を削除して post() だけにするか、もうちょいマシなのを最小限つくっておくか) |
addon-kitの各ライブラリについて、context-menuのItemはいまいちうまい拡張の仕方を思いつけなかったので、とりあえずそれ以外のDeferred化が有効そうなものについて簡単にDeferred化するようなコードを書いてみました。 |
用例です。
Page, Panel, Widgetについては単にDeferred.postie()してるだけです。 |
bind()を削除するかどうかの判断はさておき、bind()を削除する場合は、 https://github.com/cho45/jsdeferred/blob/master/binding/jetpack.js#L36 のブロックで
としておいた方がいいと思いました。そうしないとcbがどんどん大きくなってメモリリークという事になりそうな気がします。
とあるので、この局面でコールバックが1回しか呼ばれないのは仕様(by design, feature)として明言していいと思います。 ちなみに、jetpack.jsをベースにこちらでフォークして作っているJavaScriptコードモジュール向けバインディングでは、post()だけ定義してあってbind()は未実装なので、常にこのタイミングでコールバックを削除するようにしました。 |
postie で callback を delete するのはその通りですね。bind() は(まぁ見ての通りですが) あまり考えて作られてないので、とりあえず削除したいですね…… 結局あまり汎用化できないのかなあという気がしたので、piro さんのやつがベストそうですが、ある程度おおきくなりそうなので、テスト付きで別のレポジトリにしてもいいのかなあという気持ちです。 |
「関数名で役割を明示する方法」って前書きましたけど、これは実行コンテキストが違うときすごい困る (他の関数を呼べないから) のでダメですね…… |
イベントに関しては別に Deferred を使わんんでも、という気もして、単に postMessage() と addEventListener をラップし、インターフェイス自体はただの callback という何かがあったらいいのかなあと思っております (内部的に Deferred を使っても使わなくても/Deferred 使うとしたら Deferred#next が複数回よばれないことと辻褄をあわせるために、いちいち removeEventListener して loop() することになりそう) とはいえ書いてみないとわからないですね…… |
それこそ単に以下みたいな感じになりそうですね。(jQuery っぽいこういうのに慣れてるので、こういうのが良さそう)
|
JetpackでDeferred.postie()を使った時のbind()ですが、これはコールバック関数をpost()のようにコンテントスクリプトとして実行した方が便利ではないでしょうか?
具体的には、こういう事をやろうとして躓いてしまいました。
コールバック関数に引数として渡ってくるオブジェクトがDOMのイベントであることを期待してbind()を使ってみたのですが、実際には空のオブジェクト({})が渡ってきました。それで http://subtech.g.hatena.ne.jp/cho45/20110216/1297790408 を見返してみて、このコールバック関数がChrome領域のコンテキストで実行される事に気がついた次第です。
jetpack.jsの94行目でDOMのイベントオブジェクトがdeferred.call(e)で渡されていますが、これはプロセス間通信になるため、bind()のコールバック関数に渡る時には諸々の情報が欠落してしまっています。これではイベントが発火したことしか分かりません。せっかくなら、DOMのイベントを受け取って何かしらの処理ができないと意味がないのではないかと思うのですが……
というわけで、こういう用途に使えるようなバージョンとしてbindToContent()という物を書いてみました。どうでしょうか?