LoginSignup
12
11

More than 5 years have passed since last update.

@59naga

babelにPRを書いてマージされた時の話

Last updated at Posted at 2016-03-07

先々月、一月の話ですが、自分がリリースしたbabelプラグインのissueの原因を調べていたところ、原因はbabel側のプラグイン内の処理がまずいことでした。1

actual.js
export {default as foo} from "commonjs-module";
expected.js
var _foo = require("commonjs-module");

Object.defineProperty(exports, "foo", {
  enumerable: true,
  get: function () {
    return _foo.default; // undefined!
  }
});

上記の修正を、babelの公式リポジトリへPRした時のことを、手順を思い出しながら書こうと思います。

  • 修正した当時と現在で、レポジトリの開発環境がやや変わっているので、この記事はv6.6.5現在の環境で、PRを書く手順について説明します。

環境設定

まずbabel公式のリポジトリをフォークして、フォークした自分のレポジトリをgit cloneします。

babel_babel__Babel_is_a_compiler_for_writing_next_generation_JavaScript_.png

git clone https://github.com/59naga/babel
cd babel

CONTRIBUTING.md に環境設定方法が載っているで、こちらを参考にテスト環境を整えます。
以下のコマンドで、依存モジュールのインストールと初期コンパイルを一括で行います。

make bootstrap

あと、CONTRIBUTING.md の冒頭にもある通り、make bootstrapの間にでも Code of Conduct（和訳） に目を通しておいて下さい。

  • v6.6.5 現在、追加でモジュールをインストールする必要があります。2 npm install estraverse-fb

https://github.com/babel/babel/tree/master/packages
に、大まかなプラグインの一覧が載っています。

目的のコンパイル結果を担当しているプラグインを、その構文の英名3から推測するか、プラグイン内部にある/test/fixtures/spec/フォルダ内に、コンパイル前とコンパイル後のコードが定義されているので、こちらから見つけます。
あるいは、githubの上部検索メニューThis repository searchに直接コード片を入力してしまうのもありかもしれません。

今回のケースでは、babel-plugin-transform-es2015-modules-commonjsというプラグインで、以下のようなコードが生成されることを知りました。（下記はv6.4.4以前の結果です）

actual.js
export {default as foo} from "commonjs-module";
expected.js
var _foo = require("commonjs-module");

Object.defineProperty(exports, "foo", {
  enumerable: true,
  get: function () {
    return _foo.default;
  }
});

テストが通ることを確認します。4

TEST_GREP=es2015-modules-commonjs make build test
# rm -rf packages/*/test/tmp
# ...
# [02:27:37] Finished 'build' after 301 ms
# ./node_modules/.bin/kcheck
# All good! ✨
# ./scripts/test.sh
# 
#   
#   ․․․․․․․․․․․․․․․․․․․․․․․․․․
# 
#   26 passing (1s)
# 
# make test-clean
# rm -rf packages/*/test/tmp
# rm -rf packages/*/test-fixtures.json

テストは通りましたが、このコンパイル結果ではexport {default as foo} from "common-js-module";が動作しません1。

テストの追加、プラグインの修正

export {default as foo} from "common-js-module";

上記コードを動作させるためには、下記のようなコンパイル結果になるのが望ましいです。

expected.js
var _foo = require("commonjs-module");

Object.defineProperty(exports, "foo", {
  enumerable: true,
  get: function () {
    return babelHelpers.interopRequireDefault(_foo).default;
  }
});

babelHelpers.interopRequireDefault(foo)について

このコードはbabel-plugin-external-helpersを通して、最終的に下記のようなコードになります。

expected.js
var foo = require("commonjs-module");
Object.defineProperty(exports, "foo", {
  enumerable: true,
  get: function get() {
    return _interopRequireDefault(foo).default;
  }
});
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

babelでコンパイルされていない素のjsライブラリなどにはexports.__esModuleが定義されていませんので、_interopRequireDefault(foo)は{default: foo}を返します。
その結果、var expected = require('./expected.js')したさい、expected.fooからcommonjs-moduleの参照が返るようになります。

前述したexpected.jsを保存して、テストが失敗することを確認します。

TEST_GREP=es2015-modules-commonjs make build test-only
# ...
# ․․․․․․․․․․․․․․․․․․․․․․․․․․
# 25 passing (1s)
# 1 failing
#
# 1) babel-plugin-transform-es2015-modules-commonjs/interop exports from:
#     babel/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/exports-from/actual.js !== /Users/59naga/Downloads/babel/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/exports-from/expected.js
#     + expected - actual
#
#      });
#      Object.defineProperty(exports, "foo", {
#        enumerable: true,
#        get: function () {
#     +    return babelHelpers.interopRequireDefault(_foo).default;
#     -    return _foo.default;
#        }
#      });

テストを成功させるために、Object.defineProperty(exports, ...が、このpluginのsrc/index.jsのどの辺りで定義されているか目星を付けますが、行頭にbabel-templateが幾つか定義されていますので、そこからかなり目的の処理を絞れます。

src/index.js#L15
let buildExportsFrom = template(`
  Object.defineProperty(exports, $0, {
    enumerable: true,
    get: function () {
      return $1;
    }
  });
`);

buildExportsFromを使用している行が、問題の処理のようです。

babel/blob/v6.4.4/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js#L265-L276
let ref = addRequire(source.value, path.node._blockHoist);

for (let specifier of specifiers) {
  if (specifier.isExportNamespaceSpecifier()) {
    // todo
  } else if (specifier.isExportDefaultSpecifier()) {
    // todo
  } else if (specifier.isExportSpecifier()) {
    topNodes.push(buildExportsFrom(t.stringLiteral(specifier.node.exported.name), t.memberExpression(ref, specifier.node.local)));
    nonHoistedExportNames[specifier.node.exported.name] = true;
  }
}

{default as foo}のような構文の時だけ、コンパイル結果を変更します。

  • これがどのようなifを書けば良いのか分からなかったので、if(specifier.isExportSpecifier()){...}の中でconsole.log(specifier)でASTの中身を覗きました。 specifier.node.local.nameに、リネーム前の参照名が定義されていました。
babel/blob/v6.4.5/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js#L290-L304
let ref = addRequire(source.value, path.node._blockHoist);

for (let specifier of specifiers) {
  if (specifier.isExportNamespaceSpecifier()) {
    // todo
  } else if (specifier.isExportDefaultSpecifier()) {
    // todo
  } else if (specifier.isExportSpecifier()) {
    if (specifier.node.local.name === "default") {
      topNodes.push(buildExportsFrom(t.stringLiteral(specifier.node.exported.name), t.memberExpression(t.callExpression(this.addHelper("interopRequireDefault"), [ref]), specifier.node.local)));
    } else {
      topNodes.push(buildExportsFrom(t.stringLiteral(specifier.node.exported.name), t.memberExpression(ref, specifier.node.local)));
    }
    nonHoistedExportNames[specifier.node.exported.name] = true;
  }
}

specifier.node.local.nameがdefaultの時だけ、buildExportsFromの第二引数t.memberExpressionの第一引数refを、t.callExpression(this.addHelper("interopRequireDefault"), [ref])に変更します。

src/index.jsの変更点は以上です。

テストが成功することを確認します。（また、kcheck(eslint)が通ることも確認します）

TEST_GREP=es2015-modules-commonjs make build test
# ./node_modules/.bin/kcheck
# All good! ✨
# ․․․․․․․․․․․․․․․․․․․․․․․․․․
# 
# 26 passing (1s)

コミットメッセージに修正内容を簡潔に記述し、pushします。

git commit -am "Fix T6953,T2541 export-from statement renamed default issue

- Use `interopRequireDefault` helper if local name is default.

Via 59naga/babel-plugin-add-module-exports#20"
  • T***はbabelのissue管理に使われている https://phabricator.babeljs.io/ のissue番号です。
  • username/repository#issue-number のような書式をコミットメッセージに書くと、github上では自動でgithub-issueにリンクが貼られるようです。（リンク先にもログが残りました）

フォークした自分のページのNew pull requestボタンからPRを作成します。

59naga_babel__Babel_is_a_compiler_for_writing_next_generation_JavaScript_.png

件名と本文を入力してSend pull requestを押せば、あとは相手の返信を待つだけです。

スクリーンショット 2016-03-08 04.05.24.gif

git rebase -i HEAD~3
# pick yyyyyyy Modify 1
# squash zzzzzzz Modify 2
# squash 1111111 Modify 3
git push -f

という風にgit push -fで歴史を改編する必要があったことを、後になって知りました。

おわりに

手探りのため、かなり理解が怪しいですが、PRを書く参考になれば幸いです。
誤字脱字、きわどい表現などの指摘を、お待ちしております。

  1. fix前の状態でexport {default as foo} from "common-js-module";のような構文を使用すると、fooが存在しないプロパティ.defaultを参照してundefinedを返す不具合でした。CHANGELOG.md#v6.4.5(2016-01-19) ↩

  2. ./node_modules/.bin/kcheckがコケますが、内部のbabel-eslintの依存モジュール定義不足が問題です。下記のコマンドで不足したモジュールを補います。 ↩

  3. export from やら class とか function bind など… ↩

  4. Running tests - CONTRIBUTING.md, lintがやけに重いので make build test-onlyでテストだけ走らせました ↩

12
11
1

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
12
11