🌎

OSSにコントリビュートした(Vue I18n) - datetime

2022/11/03に公開

概要

Vue.jsの多言語化のためのライブラリであるVue I18nにコントリビュートをしました。
OSSへのコントリビュートを自分自身やったことがなかったので、備忘録も兼ねて記載します。
OSSコントリビュートをやってみようと思っている方の参考になれば幸いです。

対象のライブラリ

Vue-I18n

Internationalization plugin for Vue.js

issueの内容

https://github.com/kazupon/vue-i18n/issues/1502

$d doesn't support key format object as mentionned in the docs.

コントリビュートまでの実施内容

問題内容と期待動作を把握する

まず、問題の内容と期待動作を確認します。
issueを見ると、

Following the doc I expect to be able to use a Intl.DateTimeFormat inside the $d function the same way I do with $n. But $d only accept string key and object format isn't supported.
For exemple : $d(new Date(), { year: 'numeric' }) But it doesn't work and I have to define all the formats in the config first.

と書いてあります。
$dのパラメーターとしてkeyだけではなくobjectも受け取れるようにすべきという内容のようです。

開発環境を構築する

問題の内容が把握できたので、次に開発環境を構築します。
READMEを確認すると、

💪 Contribution
Please make sure to read the Contributing Guide before making a pull request.

とありますので、まずはContributing Guideの内容を見てみます。

Development Setupに、開発環境の構築手順が書いてありますのでこれに沿って進めます。

1. 開発用にビルドする

npm run dev
・・・
Error: Cannot find module 'webpack-cli/bin/config-yargs'

早速のエラー。
調べてみると、webpack-dev-serveではなく、webpack serveを使えば良さそうなのでpackage.jsonを書き換えます。
https://github.com/webpack/webpack.js.org/pull/4091/files

package.json
- "dev": "cross-env BABEL_ENV=test webpack-dev-server --inline --hot --open --content-base ./test/unit/ --config config/webpack.dev.conf.js",
+ "dev": "cross-env BABEL_ENV=test webpack serve --inline --hot --open --content-base ./test/unit/ --config config/webpack.dev.conf.js",

修正したところ、無事にunit testが完了しました。

unit test

2. lintを実行する

npm run lint

warningは出るようですがとりあえずOK。

3. ユニットテストを実行する

npm run test:unit
・・・
Safari 16.0 (Mac OS 10.15.7): Executed 109 of 312 (2 FAILED) (0 secs / 0.443 secs)
Safari 16.0 (Mac OS 10.15.7): Executed 312 of 312 (3 FAILED) (3.213 secs / 2.661 secs)
TOTAL: 5 FAILED, 1044 SUCCESS

SafariでFAILEDとなっているようです。

結果を詳しく見ると、

Safari 16.0 (Mac OS 10.15.7) basic i18n#n locale argument with second argument should be formatted FAILED
          # test/unit/basic.test.js:799
          
          assert.strictEqual(i18n.n(money, 'currency', 'ja-JP'), '¥10,100')
                             |    | |                                       
                             |    | 10100                                   
                             |    "¥10,100"                                 
                             VueI18n{_vm:#Vue#,_formatter:#BaseFormatter#,_modifiers:#Object#,_root:null,_sync:true,_fallbackRoot:true,_fallbackRootWi

となっており、$nでのエラーでした。
今回修正をしたい$dには関係がなさそうなのと、解決策がすぐにはわからなかったため一旦先に進んでみます。

4. ビルドする

npm run build

問題なく完了。

5. すべてのテストを実行する

npm run test
・・・
OK. 66  total assertions passed (37.324s)

テストOKです。
npm run test:unitでの一部問題はあったものの一旦はこれで進められそうです。

6. exmampleを確認する

exampleというディレクトリがあったのでこれの中をみるとdatetime/index.htmlというファイルがあります。
これをブラウザで開いて見てみると$dの実行結果が表示されます。これを使って開発を進めていけば良さそうです。
$dのexample

これで開発できる環境は整いました。

修正方針を決める

開発環境が整いましたのでどのように修正をしてくか考えます。
あらためてissueを確認してみますと、

Following the doc I expect to be able to use a Intl.DateTimeFormat inside the $d function the same way I do with $n

との記載があり、$nにはある機能が$dにはないと言っているようです。

index.js$nを見てみますと、

index.js
n (value: number, ...args: any): NumberFormatResult {
  ・・・
  if (args.length === 1) {
    ・・・
    } else if (isObject(args[0])) {
      ・・・
      // Filter out number format options only
      options = Object.keys(args[0]).reduce((acc, key) => {
        if (includes(numberFormatKeys, key)) {
          return Object.assign({}, acc, { [key]: args[0][key] })
        }
        return acc
      }, null)
    }
  }
  ・・・
  return this._n(value, locale, key, options)
}

となっており、$nの2つ目の引数がObjectだった場合にoptionsに変換する処理があります。
この処理が$dにはないようなので、同じような処理を$dに書いていけば良さそうです。

修正する

あとは$d$nを見比べて、optionsに関する処理を$dに追加していきます。

修正が一通り完了したあとに動作チェックのために、datetime/index.html$dの引数にobjectを渡してみますと、

datetime/index.html
<p>{{ $t('current_datetime')}}: {{ $d(now, 'long') }}</p>
+ <p>{{ "年(2桁)" }}: {{ $d(now, {year: '2-digit'}) }}</p>

以下のように表示され、無事動きました。
動作確認結果

テストコードを書く

最後に、テストコードを追加します。

1. テストコードの追加

testディレクトリを確認すると、e2eの中にnumber.jsはありますが、datetime.jsがありません。
number.jsを参考にdatetime.jsを追加するのが良さそうです。

2. テストケースの作成

/test/e2e/test/number.jsを見るとexamplesの下のファイルを実行しているようですので、/examples/number/datetime/index.htmlも修正してテストケースを追加するのが良さそうです。

number.js
module.exports = {
  number: function (browser) {
    browser
      .url('http://localhost:8080/examples/number/')
      .waitForElementVisible('#app', 1000)
      .assert.containsText('p', 'お金: ¥1,000')
      .click('select option[value=en-US]')
      .assert.containsText('p', 'Money: $1,000')
      .end()
  }
}

3. タイムゾーンの固定

テストコードを追加して無事に動きましたが、日時のテストですので実行環境によらずテストできるようにタイムゾーンは固定したいです。

e2eにはNightwatch.jsを使っているようなので、Nightwatch.jsのサイトを見てみますと

nightwatch.json
module.exports = {
    ...,
    test_settings: {
        ...,
        saucelabs: {
            desiredCapabilities: {
                ...,
                timeZone: 'London'

とあるので、この設定でできそうです。

と思ってやってみましたが、、、timeZoneが設定されず、他の方法を考えます。

そこで、既存のテストでも何らかの方法でタイムゾーンは固定してるはずだと思い、他のコードを見てみると/test/unit/fixture/datetime.jsにありました!

if (isChrome) {
  formats['en-US']['short']['timeZone'] = 'America/New_York'
  formats['ja-JP']['short']['timeZone'] = 'Asia/Tokyo'
  formats['ja-JP']['long']['timeZone'] = 'Asia/Tokyo'

$dの機能自体にtimeZoneというオプションがあり、これで設定できるようです。

これでexampleのファイルを以下のように修正すれば良さそうです。

index.html
      var dateTimeFormats = {
        'en-US': {
          long: {
            year: 'numeric', month: '2-digit', day: '2-digit',
            hour: '2-digit', minute: '2-digit', second: '2-digit',
+            timeZone: 'America/Los_Angeles'
          }
        },
        'ja-JP': {
          long: {
            year: 'numeric', month: '2-digit', day: '2-digit',
            hour: '2-digit', minute: '2-digit', second: '2-digit',
            hour12: true,
+            timeZone: 'Asia/Tokyo'
          }
        }
      }

PRを出す

これで修正が一通り終わったので、Pull Request Guidelinesを見て、書いてあるとおりにPRを出します。

PRのコメントは特に指定のフォーマットがなかったので、修正方針やスクリーンショットを貼ってわかりやすくしておきました。
https://github.com/kazupon/vue-i18n/pull/1569

翌日にはすぐにレビューいただいて無事マージされました!

GitHubで編集を提案

Discussion