Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

イベントとモデル: TodoListModel.js の onChange の返している関数の意味 #607

Closed
tabass opened this issue Dec 21, 2018 · 2 comments · Fixed by #620
Closed
Assignees

Comments

@tabass
Copy link

tabass commented Dec 21, 2018

該当ページ

https://jsprimer.net/use-case/todoapp/event-model/#event-emitter-todolist-model

質問内容

src/model/TodoListModel.jsonChangeは以下のようになっていますが、

    onChange(listener) {
        this.addEventLister("change", listener);
        return () => {
            this.removeEventLister("change", listener);
        };
    }

単に以下のような実装でも正常に動作するように思いました。

    onChange(listener) {
        this.addEventLister("change", listener);
    }

returnremoveEventListerを呼ぶ関数を返しているのには、どのような意味があるのか教えていただけますでしょうか。

@azu
Copy link
Collaborator

azu commented Dec 22, 2018

const unlisten = onChange(() => { /* onChangeのハンドラ */ });
// onChangeのハンドラを解除する
unlisten();

というパターン向けにremoveEventLister(登録したイベントリスナーを解除する処理)を返していた感じですね。
登録したイベントは解除しないとメモリリークの原因となるので、ちゃんと解除できるように作るのがいいのですが、正直このサンプルだとリロードで十分なのでそこまでいけてないという感じですね。

ホントは mountaddEventLister して

this.todoListModel.onChange(() => {

unmountremoveEventLister するという対比構造にする予定で書いていたのですが、
unmountの部分が省略されてしまった残骸みたいなものになっていますね。
return () => this.removeEventLister("change", listener); の部分は消そうかなと思います。

@tabass
Copy link
Author

tabass commented Dec 23, 2018

ご回答ありがとうございます。本書のレベルは私にちょうど良く、たいへん勉強になっています。おかげさまで上記のやり取りが理解できるほどになってきたかと思います。ありがとうございます。

@azu azu self-assigned this Dec 29, 2018
azu added a commit that referenced this issue Dec 30, 2018
TODOアプリのユースケースでは解除まで行っていないので削除する

fix #607
@azu azu closed this as completed in 86a7ceb Dec 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants