DefinitelyTypedの型定義を更新するPullRequestしてみた話

使っていますか＠types。便利ですよね。
あの型定義ファイルを提供しているのが、DefinitelyTypedです。
有志がもりもり書いてくれる資源に乗っかっていく。素晴らしきかな。

ただ今回、＠typesからインストールした型定義ではうまくコンパイルが通らないコードがあり、簡単に修正できそうだったので、せっかくなので自分もPullRequestにチャレンジしてみました。

この記事はそのレポートみたいなやつです。
正直よくわからないまま勢いでやったので、ツッコミどころがある方はどうぞご指導ください。

コンパイルエラー出たやつ

strictNullChacksを有効にしているプロジョクトで、expressjs/multerを使っている際に、
以下のようなコンパイルエラーが発生しました。

StampAPI.ts

// multerミドルウェアの初期化
this.upload = multer({
    // アップロードするファイルの保管先
    dest: "stamps/",
    // 送られてきたファイルが、別途設定してあるMIMEタイプに該当すれば受け入れる
    fileFilter: (req, file, callback) =>
        callback(null, _.some(StampAPI.acceptableMimeTypes, file.mimetype))
});

error TS2345: Argument of type 'null' is not assignable to parameter of type 'Error'.

「nullはError型じゃねぇよ！」と怒られてしまいますね。

このfileFilterは、エラーを第一引数にとるコールバック関数です。
何らかのエラーを伝える必要がある場合はここにErrorオブジェクトを渡し、問題なく処理ができた場合はnullを指定します。使い方としてはこれで間違っていないはずです。
multerのREADMEにも、nullを代入する例が記載されています。

エラー原因

node_modules/@types/multer/index.d.tsにおける、この関数の定義を確認してみましょう。

index.d.ts

/** A function to control which files to upload and which to skip. */
fileFilter?(req: Express.Request, file: Express.Multer.File, callback: (error: Error, acceptFile: boolean) => void): void;

コールバック関数の第一引数の型は、単にErrorとなっています。
今回のプロジェクトではstrictNullChecksを有効にしているため、これではnullを代入できません。まあそのオプションを外せば通るんですけれど、せっかくなので。
型定義は実際の用途に即しているべきだと思いますし。

修正してやる！！！

index.d.ts

/** A function to control which files to upload and which to skip. */
fileFilter?(req: Express.Request, file: Express.Multer.File, callback: (error: Error | null, acceptFile: boolean) => void): void;

コールバックの第一引数の型を、ErrorからError | nullのUnion Typeに変更し、nullを許容するということを明示しました。これで、strictNullChecksでも問題なくコンパイルできるようになります。
多分これでオッケーな気がするので、プルリクエスト飛ばしてみましょう。

PullRequestしよう

Contribution guide | DefinitelyTyped

ガイドラインが用意されていたので、頑張って読めるだけ読みました。
今回のケースでは、既存のファイルに対して僅かな変更を加えるだけなので、大半は飛ばしても問題なさそうです。
とりあえず、テスト周りは多分ちゃんと走らせました。（npm test）
ついでに、変更箇所のテストするためのテストケースも一応追加。
（これもうちょっとちゃんと書いておいたほうが良かった気がするけどまあいいや）

テンプレートうめる

いざPR飛ばそうとしたら、テンプレートでめっちゃチェックボックスがあります。
ちゃんと諸々確認してからプルリク飛ばして来ようね、ということでしょう。
このテンプレートでは、

• 定義を追加した
• 既存の定義に変更を加えた
• 定義を削除した

というケースに応じて、必要なチェックボックスを残して、クリアしてそうな気がしたらチェックつけておきます。そんな気がする。これでよかったんだろうか……。

自動テストとなんやかんや

PRを飛ばすと、Travis CIによる自動テストが走るので、しばし待つことになりました。
少し後にリロードすると、無事テストクリアしています。やったね。
あとはDefinitelyTypedのメンバーに取り込んでもらえばオッケーなはず。

その際、なんか既存のauthorにレビューしてもらいますか？とかBotが言っている気がしましたが、日和って無視してました。英語わからんこわい。多分これかのどちらかしなきゃいけないんでしょうね。自分が日和っている間に、DTの親切な方が、代わりにauthorの人たちに「コメントある？特になければマージします」って聞いてくれたりしてくれたみたいです。やさしい。ごめんなさいありがとうございます。

無事マージ

途中、コンフリクトが発生したので解消したりしましたが、ちゃんと取り込まれました。やったね。

参考にさせていただいた記事

• TypeScriptの型定義ファイルを共有しよう！
  http://qiita.com/vvakame/items/1980d4b6cc222e03fdcb
• typescriptの定義ファイルd-tsを実際にdefinitelytypedにpullrequestしてみた
  http://qiita.com/kenfdev/items/0990f081030f7d710242