Skip to content
This repository was archived by the owner on Jan 19, 2018. It is now read-only.

Bind to content #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Bind to content #11

wants to merge 7 commits into from

Conversation

piroor
Copy link
Contributor

@piroor piroor commented Mar 1, 2011

JetpackでDeferred.postie()を使った時のbind()ですが、これはコールバック関数をpost()のようにコンテントスクリプトとして実行した方が便利ではないでしょうか?

具体的には、こういう事をやろうとして躓いてしまいました。

var data    = require('self').data;
var widgets = require('widget');
var panels  = require('panel');
var tabs    = require('tabs');

var Deferred = require('jsdeferred.jetpack').Deferred;

var eraserWidget = Deferred.postie(widgets.Widget, {
      label      : 'ダブルクリックした所を消す',
      contentURL : data.url('icon.png'),

      onClick    : function() {
        // 現在のページをパネルで開く。
        // このパネルではダブルクリックした所にある要素を消せる。
        var panel = Deferred.postie(panels.Panel, {
              contentURL : tabs.activeTab.url,
              width      : 1024,
              height     : 768
            });
        panel.bind('body *', 'dblclick', function(aEvent) {
          var node = aEvent.target;
          if (node.nodeType != node.ELEMENT_NODE)
            node = node.parentNode;

          var ready = 'data-ready-to-erase';
          if (node.hasAttribute(ready)) {
            node.parentNode.removeChild(node);
          }
          else {
            node.setAttribute(ready, true);
            node.style.setProperty('outline', '2px solid red', 'important');
            Deferred
              .wait(3)
              .next(function() {
                node.removeAttribute(ready);
                node.style.outline = '';
              });
          }
        });
        panel.show();
      }
    });

コールバック関数に引数として渡ってくるオブジェクトがDOMのイベントであることを期待してbind()を使ってみたのですが、実際には空のオブジェクト({})が渡ってきました。それで http://subtech.g.hatena.ne.jp/cho45/20110216/1297790408 を見返してみて、このコールバック関数がChrome領域のコンテキストで実行される事に気がついた次第です。

jetpack.jsの94行目でDOMのイベントオブジェクトがdeferred.call(e)で渡されていますが、これはプロセス間通信になるため、bind()のコールバック関数に渡る時には諸々の情報が欠落してしまっています。これではイベントが発火したことしか分かりません。せっかくなら、DOMのイベントを受け取って何かしらの処理ができないと意味がないのではないかと思うのですが……

というわけで、こういう用途に使えるようなバージョンとしてbindToContent()という物を書いてみました。どうでしょうか?

@cho45
Copy link
Owner

cho45 commented Mar 2, 2011

なるほどー

bind は気持ち的にはクリックしたことだけうけとって、さくっと chrome 側の処理を追加したい! みたいな感じでつくったのでそうなってしまってました。(content 用のイベントは post() 内で普通に addEventListener() してしまう気持ち)

bindToContent 自体はいれてしまおうと思います! ありがとうございます!

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

あ、やはりそうでしたか。
個人的にはaddEventListener/removeEventListenerのペアをなるべく書きたくない・そういう煩雑な部分こそラップされていて欲しい部分だったので、むしろこういうユーティリティも作ってました。

function waitDOMEvent(aTarget, aType, aUseCapture) {
  var deferred = new Deferred();
  aTarget.addEventListener(aType, function(aEvent) {
    aTarget.removeEventListener(aType, arguments.callee, aUseCapture);
    deferred.call(aTarget);
  }, aUseCapture);
  return deferred;
}

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

ちょっと話がずれるのですが、bind()の具体的なユースケースというのはどういうものが想定されているんでしょうか?
もし監視するイベントの種類が限定されるのならば、第2引数でイベント型を指定するという仕様ではなく、
widget.onClickOn(selector, callback).next() { ... })
といった感じで、「クリックしたことだけうけとって、さくっと chrome 側の処理を追加」という使い方に誘導されるようなAPIになってると、初めて使う人には嬉しいのではないかと思いました。

今のbind()はメソッド名や引数に取れる内容、あと、用例でもコールバック関数がイベントらしきオブジェクトを受け取っているように見えることから、汎用的に使える事を期待させてしまうインターフェースになってしまっているので、自分のような勘違いをしてしまう人が出てくるんじゃないか……という気がしています。

bindToContent()はネーミング的にJSDeferredの既存の文法(メソッド名は基本的に1語で、存在をあまり意識させない)に則っていないので、何かいい名前が欲しいですね。

@cho45
Copy link
Owner

cho45 commented Mar 2, 2011

確かにそうですね > onClickOn みたいな
ちょっといろいろいじってみます

@cho45
Copy link
Owner

cho45 commented Mar 2, 2011

post() はコンテントの中で実行されている感が多少名前からわかりますが、単に click() とかだと関数が chrome で実行されるのか content で実行されるのかわからないですね……

widget に追加するメソッドは全て content で実行されるように統一して、next() で繋げると chrome 側に処理が渡るとかでしょうか…… 通常の Deferred だと next() に入れた関数が複数回実行されることを想定しないのでこれはこれで気持ち悪いですが……

widget.click(selector, function (e) {
   // content context
  return "foobar";
}).
next(function (data) {
  // chrome context
  alert(data); //=> "foobar"
});

あるいは中間オブジェクトをはさんで jQuery っぽくしつつ、さらに関数名でコンテキストを明示するという気持ち悪い方法とか…… (これだと click() とかの返り値を Deferred にできない)

関数名で役割を明示する方法は既に Deferred.chain() というので存在してるのでそんなに悪いとは思ってないんですが、普通に考えるとキモい。

widget.find(selector).
  click(function content (e) {
    // run in content
  }).
  click(function chrome () {
    // run in chrome
  });

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

あるいは中間オブジェクトをはさんで jQuery っぽくしつつ、さらに関数名でコンテキストを明示するという気持ち悪い方法とか…… (これだと click() とかの返り値を Deferred にできない)

うおお……これは確かにJSDeferredらしくないですね。

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

ぼんやり考えてたのですが、Deferredそのものが持つネイティブな機能と、WidgetやPanelに追加される便利機能とは、色々分けて考えた方がいいのかなと思いました。
もっと突き詰めると、Deferred.pstie()でコンストラクタ関数を拡張する代わりに、

var jsdeferred = requre('jsdeferred.jetpack');
var Deferred = jsdeferred.Deferred;
var DeferredWidget = jsdeferred.Widget;
var myWidget = DeferredWidget({ ... });

のような感じであらかじめ機能を拡張した物を提供する(そこまでがJSDeferredのJetpackバインディングの機能と考える)とか。
この考え方だと、widget.bindToContent(...)という風な少々JSDeferredらしくないネーミングでも、個人的には許容できます。
(でもこういう方向で頑張るんだったらJSDeferred本体とは別のプロジェクトとしてやった方がよさそう)

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

今のJSDeferredをベースにすると、実装としてはこんな感じでしょうか……

exports.__defineGetter__('Widget', function() {
    delete this.Widget;
    var Widget = require('widget').Widget;
    return this.Widget = function(options) {
        return Deferred.postie(Widget, options);
    };
});

exports.__defineGetter__('Panel', function() {
    delete this.Panel;
    var Panel = require('panel').Panel;
    return this.Panel = function(options) {
        return Deferred.postie(Panel, options);
    };
});

@cho45
Copy link
Owner

cho45 commented Mar 2, 2011

リッチなのはリッチなのであってもいいですが最低限のだけつけときたいですね (bind() を削除して post() だけにするか、もうちょいマシなのを最小限つくっておくか)

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

addon-kitの各ライブラリについて、context-menuのItemはいまいちうまい拡張の仕方を思いつけなかったので、とりあえずそれ以外のDeferred化が有効そうなものについて簡単にDeferred化するようなコードを書いてみました。
どうでしょうか?

https://github.com/piroor/jsdeferred/blob/f1eaeb40d14ad7be5187235c27fa6e75d3d071a9/binding/jetpack.js

@piroor
Copy link
Contributor Author

piroor commented Mar 2, 2011

用例です。

var data    = require('self').data;
var jsdeferred = require('jsdeferred.jetpack');

var Deferred = jsdeferred.Deferred;

// *.orgなサイトに適用するPageModを定義
jsdeferred.PageMod({ include : "*.org" })
  .next(function(aWorker) {
    // 該当するページが読み込まれた時にここに処理が回ってくる。
    // Deferred.postie()済みのワーカーオブジェクトが渡される。
    // (ワーカーオブジェクトにDeferred.postie()するために、
    // コンストラクタ関数以外も受け付けるようDeferred.postie()を拡張してある)
    return aWorker
            .post(function() {
              // この内容はコンテントスクリプトとして実行される。
              content.alert('.org page is loaded!');
              return Deferred.next(function() {
                // ここでもDeferredチェインを使える。
                return content.location.href;
              });
            });
  })
  .next(function(aURI) {
    // コンテントスクリプトからのチェインがここに繋がる。
    dump(aURI+'\n');
  });

jsdeferred.Widget({
  label      : 'request test',
  contentURL : data.url('icon.png'),
  onClick    : function() {
    // Requestを投げる先を指定。
    // この時点で、返り値は実はDeferredオブジェクト。
    jsdeferred.Request({ url: "http://api.twitter.com/1/account/rate_limit_status.json" })
      .get() // Requestのget/setをラップしてDeferredオブジェクトを返すメソッド。
      .next(function(aResponse) {
        // onCompleteのタイミングでここに処理が回ってくる。
        // onCompleteに渡るはずだった内容がここに渡ってくる。
        dump(uneval(aResponse.json)+'\n');
      });
  }
});

Page, Panel, Widgetについては単にDeferred.postie()してるだけです。

@piroor
Copy link
Contributor Author

piroor commented Mar 4, 2011

リッチなのはリッチなのであってもいいですが最低限のだけつけときたいですね (bind() を削除して post() だけにするか、もうちょいマシなのを最小限つくっておくか)

bind()を削除するかどうかの判断はさておき、bind()を削除する場合は、 https://github.com/cho45/jsdeferred/blob/master/binding/jetpack.js#L36 のブロックで

   c(message.value, message.error);
 + delete cb[message.id]
   return;

としておいた方がいいと思いました。そうしないとcbがどんどん大きくなってメモリリークという事になりそうな気がします。

Deferred だと next() に入れた関数が複数回実行されることを想定しないので

とあるので、この局面でコールバックが1回しか呼ばれないのは仕様(by design, feature)として明言していいと思います。
bind()を残す場合は、post()がbind()経由で呼ばれた時はコールバックを削除せず、直接呼ばれた時は削除する、という風にするのが現実的でしょうか。

ちなみに、jetpack.jsをベースにこちらでフォークして作っているJavaScriptコードモジュール向けバインディングでは、post()だけ定義してあってbind()は未実装なので、常にこのタイミングでコールバックを削除するようにしました。
piroor@54a25f9

@cho45
Copy link
Owner

cho45 commented Mar 4, 2011

postie で callback を delete するのはその通りですね。bind() は(まぁ見ての通りですが) あまり考えて作られてないので、とりあえず削除したいですね……

結局あまり汎用化できないのかなあという気がしたので、piro さんのやつがベストそうですが、ある程度おおきくなりそうなので、テスト付きで別のレポジトリにしてもいいのかなあという気持ちです。

@cho45
Copy link
Owner

cho45 commented Mar 4, 2011

「関数名で役割を明示する方法」って前書きましたけど、これは実行コンテキストが違うときすごい困る (他の関数を呼べないから) のでダメですね……

@cho45
Copy link
Owner

cho45 commented Mar 4, 2011

イベントに関しては別に Deferred を使わんんでも、という気もして、単に postMessage() と addEventListener をラップし、インターフェイス自体はただの callback という何かがあったらいいのかなあと思っております (内部的に Deferred を使っても使わなくても/Deferred 使うとしたら Deferred#next が複数回よばれないことと辻褄をあわせるために、いちいち removeEventListener して loop() することになりそう)

とはいえ書いてみないとわからないですね……

@cho45
Copy link
Owner

cho45 commented Mar 4, 2011

それこそ単に以下みたいな感じになりそうですね。(jQuery っぽいこういうのに慣れてるので、こういうのが良さそう)

var widget = ...;
widget.find('input.button-a').
  click("content", function () {
  }).
  click("chrome", function () {
  })

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants