Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Feature #4918: Upgrade Core Components #4919

Conversation

mcdmaster
Copy link
Contributor

@mcdmaster mcdmaster commented Jun 30, 2020

👏 解決する issue / Resolved Issues

📝 関連する issue / Related Issues

⛏ 変更内容 / Details of Changes

  • Upgrade to core-js3
  • Upgrade to @vue/server-renderer
  • ...and many more!

📸 スクリーンショット / Screenshots

(Update in Jul 3)

Before, with many warnings:
コメント 2020-07-03 063045

After, with less warnings:
コメント 2020-07-03 062410

@mcdmaster mcdmaster marked this pull request as ready for review July 1, 2020 07:28
@goki90210
Copy link
Contributor

@mcdmaster
package.jsonの
差分で確認したいことがあります。

    "vue": "^2.6.11",
    "@vue/server-renderer": "^3.0.0-beta.15",

で矛盾があります。@vue/server-rendererの3.xを適用する場合vueも3.xでないとワーニングです。

warning " > @vue/server-renderer@3.0.0-beta.17" has incorrect peer dependency "vue@3.0.0-beta.17"

これで良いのでしょうか?

以下は注釈です。

  • babelは6月30日に7.10.4がリリースされたようです。
    https://github.com/babel/babel

  • ESLintに関しては^6.8.0でも^7.3.1でもワーニング出ることがわかっているので(私の中では)無視しています。

@kaizumaki
Copy link
Collaborator

@mcdmaster ご提案ありがとうございます!少し教えてください 🙏 こちらは依存パッケージをpackage.jsonに明示的に書いておくことで、このリポジトリに関係するものをアップデートしたい、という意図でしょうか?かなりのパッケージがpackage.jsonに追記されているので気になっています。よろしければ、どういう手順(コマンド)で追記するに至ったか教えてもらえるとうれしいです。

@mcdmaster
Copy link
Contributor Author

@goki90210 @kaizumaki コメントありがとうございます。
私はかねてから deprecated の表示がいちいち気になっていましたので、原始的な方法で deprecated の表示がこれ以上減らないところまで package.json ファイルの足し引きをしました。
ご指摘に基づけば、まだまだいじる余地はありそうですね

@goki90210
Copy link
Contributor

VueがBeta版なのですが、Beta版でも採用したい理由がなにかあるのでしょうか?

devDependenciesにBeta版を入れるのはまだしも、dependenciesにベータ版を含めるのはちょっと抵抗があります。

@mcdmaster
Copy link
Contributor Author

@goki90210 失礼しました。ちょっとした手違いです。
dependenciesdev でない方)に、ベータ版が紛れ込んでいたものをコミットしてしまっていました

@goki90210
Copy link
Contributor

goki90210 commented Jul 2, 2020

@mcdmaster

"vue": "^2.6.11",

を消しても解決しません。

"@vue/server-renderer": "^3.0.0-beta.17",

をdevDependenciesに入れることで、

"vue": "^3.0.0-beta.17",

も参照することになります。

Vueは多くのソースで参照しているので、dependenciesに入れるべきものであると考えます。
(理由は https://rules.sonarsource.com/typescript/RSPEC-4328 で、Issue #4871 PR #4875 で対応したものです。)

ということで、暗黙的にdependenciesに

"vue": "^3.0.0-beta.17",

を入れているのと同じになります。

"@vue/server-renderer": "^3.0.0-beta.17",

を入れたい理由は何でしょうか?

@mcdmaster mcdmaster force-pushed the feature/4918-Upgrade-Components branch from d5de4d8 to 40e6a47 Compare July 2, 2020 21:55
@mcdmaster
Copy link
Contributor Author

mcdmaster commented Jul 2, 2020

@vue/server-renderer も含め、deprecated というワーニングが出たものについては可能な限り解決を図ることを目指しました。
なぜなら、最新パッチを常に適用することは IT セキュリティの観点からは不可避という前提を置いたからです。

昔日の Microsoft のパッチのように、その適用がユーザビリティを損ねた深刻な歴史も知っています。21 世紀の IT においては、MS のようなプロプライエタリでないオープンソース・ソフトウェアがプロダクションに乗る際にはパッチ評価の活動もコミュニティができるところはやる、というふうになってくると思いました

@goki90210
Copy link
Contributor

@mcdmaster
順を追って具体的に説明しましょう。

まず、core-js@3にすべき/したいのは理解してます。(これがきっかけですよね?)
これはDeprecatedというよりもcore-js@2は複数の問題があるにもかかわらずメンテナンスされていないのでcore-js@2に依存しているものの使用はやめcore-js@3に置き換えましょうということです。

core-js@2に依存しているのは yarn listでしらべると

  • @nuxt/babel-preset-app

です。
@nuxt/babel-preset-appについてはBabelを7.4.0以上にすることでcore-js@3を使用するオプションが使用できます。
https://www.npmjs.com/package/@nuxt/babel-preset-app

ということで、現在のプロジェクトに
> yarn add -D @babel/core core-js@3 @babel/runtime-corejs3
を実行します。
この他になにかありますか?

@vue/server-rendererについては私はそうしなければならない文献にたどり着けていません。
何度も書いていますが、Vue 3についてはBeta版なので将来の導入のための準備や、実験的に導入することに関しては反対しませんが、正式リリースのものに含めることについては躊躇せざるを得ません。

@kaizumaki
Copy link
Collaborator

@mcdmaster @goki90210 ご検討いただきありがとうございます。
このPRで追加された依存ライブラリが手作業で yarn add してなされたものであるなら、それが多数発生していることが気がかりです。将来的に、このようなメンテナンス作業を続けていくことを持続できるかなと少し心配しています。
例えばそれが yarn audit などのコマンドを使ってメンテナンスできるなら、その手順をドキュメント化するというのでもいいと思います。
的外れなコメントでしたらごめんなさい。

@goki90210
Copy link
Contributor

@kaizumaki
core-js@3の導入については対応するための手順がいろいろあるようなので、
問題が発生しないかどうかのチェックは必要であろうと思います。

#4932 については
yarn upgrade-interactiveのコマンド結果を受けてupgradeを行うつもりです。
NuxtJS, nuxt-i18n, typescript(, dayjs)ともにバグフィックスのみです
(詳細は #4932 に記載します)

#4923 については
NuxtJSのバージョンアップに伴う追加の対応です。

@goki90210
Copy link
Contributor

yarn auditの結果を見ると
ほぼ、lodashの指摘(level: Low)だけで5244個出てしまって
> yarn audit --level moderate
をしないと結果の確認自体がつらいですね…。

@goki90210
Copy link
Contributor

@mcdmaster
core-js@3導入の件に戻ります。
NuxtJSでcore-js@3を使用するには、nuxt.config.tsを修正する必要があります。
(参考) https://qiita.com/usu_blog/items/61fea77d688243ebbb4a
※上記URLではie: '11'の記載がありますが当プロジェクトではie11はサポート対象外なので外しました。
(参考) https://www.aizulab.com/blog/referenceerror-regeneratorruntime-is-not-defined/

(前略)
  build: {
    babel: {
      presets: [
        [
          '@babel/preset-env',
          {
            "targets": {
              "node": "current"
            },
            useBuiltIns: 'usage',
            corejs: 3
          }
        ]
      ]
    },
    postcss: {
(後略) 

ここまでで、core-js@3導入に関しては作業完了のはずです。

core-js@3の導入と、それ以外(Vue 3の導入など)は分けませんか?

@mcdmaster
Copy link
Contributor Author

アドバイスを参考に、core-js@2.6.11, vue@2.6.11 でまとめてみました。
ちなみに、@vue/server-renderer には実はそれほどこだわりはありませんでした。
むしろ、corejs2 をはじめとして request@2.88.2resolve-url, urix などのモジュールの deprecated ワーニングの撲滅を狙っていました

@goki90210
Copy link
Contributor

@mcdmaster
これだとcore-js@2のままになりませんか?
core-js@3は入れたほうがいいと思うのですが…。

@mcdmaster
Copy link
Contributor Author

@goki90210 もしかしたら私が過去コメントを読み違えているかもしれないので、為念確認です。
いただいたコメントの実装がされれば、core-js3 の実装は done という理解(合意)でよいのですね。
であれば、実施します。ちなみに、私は特に異存はありません

@goki90210
Copy link
Contributor

@mcdmaster

いただいたコメントの実装がされれば、core-js3 の実装は done という理解(合意)でよいのですね。
であれば、実施します。ちなみに、私は特に異存はありません

はい。core-js@3の導入は(解決策は様々なところで事例があるので)問題はない認識です。

アドバイスを参考に、core-js@2.6.11, vue@2.6.11 でまとめてみました。
ちなみに、@vue/server-renderer には実はそれほどこだわりはありませんでした。
むしろ、corejs2 をはじめとして request@2.88.2resolve-url, urix などのモジュールの deprecated ワーニングの撲滅を狙っていました

request@2.88.2, resolve-url, urix は、webpackやjestが依存しているライブラリの問題ですよね…。

  • jest > jest-cli > jest-config > jest-environment-jsdom > jsdom > request@2.88.2
  • vue-jest > extract-from-css > css > urix@0.1.0
  • nuxt > @nuxt/webpack > webpack > micromatch > snapdragon > source-map-resolve > urix@0.1.0
  • nuxt > @nuxt/webpack > webpack > micromatch > snapdragon > source-map-resolve > resolve-url@0.2.1

本筋としては、

  1. css, jsdom, source-map-resolveがこれらのライブラリを使用しないようにする。
  2. extract-from-css, snapdragon, jest-environment-jsdom が css, source-map-resolve, jsdomを使用しないようにする。
  3. 以下これらより上位のライブラリが下位のライブラリを使用しないように…
  4. (以下繰り返しでどうしようもなくなった場合に限り)当PJで対応
    の順だと思います。

またvue-jest, jestはこのプロジェクトでは使っていないので、
外してしまってもビルドは通ります。
(将来的にjestを使って単体テストを行いカバレッジレポートを含めるということであれば話は別ですが…)
vue-jestやbabel-jestをpackage.jsonに含めた経緯が私は知らないのでなんとも言えません。
とにかく、jestを実行しない限り問題にならないと思われます。

source-map-resolveってSource Mapを作るためのライブラリですよね?
このPJで使っていないのであれば対応しなくても問題はないのではないでしょうか?

ということで、
request@2.88.2, resolve-url, urixの対応はこのPJとしては対応する優先度はかなり低いと考えます。

@kaizumaki
Copy link
Collaborator

@mcdmaster @goki90210 議論が進んでいて、ありがたい限りです。
このPRはもう少し検討したほうがいいように見受けますが、どうでしょう?先に #4933 を採用しようと思いますが問題ないでしょうか?

@mcdmaster
Copy link
Contributor Author

@kaizumaki 賛成です。ぜひ #4933 から進めていただければと。

私がやっている request@2.88.2, resolve-url, urix といったあたりのワーニングの解消は、たしかに @goki90210 さんがおっしゃるようにサイトの目的から見れば優先度は低いと思います。
とはいえ、あたかも野外市場のように入退出自由なオープンソース開発プロジェクトで、ワーニングがドカドカ出るのをあまり心地よく思わない人もいるのでは、と感じたのが作業を始めたキッカケです。
ちなみに、私も「あまり心地よく思わない人」のグループに入ります(笑)

@goki90210
Copy link
Contributor

@mcdmaster

ちなみに、私も「あまり心地よく思わない人」のグループに入ります(笑)

それ自体に楯突いているわけではないのです。
(私も本心はこういうのが残るのは嫌な人です。)

mcdmaster さんがやろうとされていることが見える化されていないのが、只々残念なのです。
ブラックボックスです。

  • core-js@3をなぜ入れないといけないのか。(これについては私がここで書きましたがIssueの方には載っていません。)
  • request@2.88.2, resolve-url, urix といったあたりのワーニングの解消についても私が聞くまで説明がありませんでした。(単にDeprecatedだとしか書かれていません。)
  • request@2.88.2, resolve-url, urixを使用しないようにすることに対してどういうことを行ったのか説明がなく、package.json, yarn.lockの差分だけの提示になっていてプログラマー以外の方には何をやっているのか理解しにくいのです。

Deprecatedを解消するにあたっての

  • 対象
  • 対処方法
    を解説いただけないでしょうか?

@mcdmaster
Copy link
Contributor Author

mcdmaster commented Jul 5, 2020

うーん、私は盾突かれているとも思っちゃいないのですけどね。
むしろ、 @kaizumaki さんもおっしゃっているように、いろいろと議論ができるのがとても有意義と感じています。
それは、 @goki90210 さんが辛抱強く議論を導いていただけるからでもあります。

ターゲット(スコープ)の絞り込みすぎ、あるいはノウハウの属人化を避ける意味で、以下2点については説明させてください。
・対象:たとえば yarn install コマンドを打った後のビルド・ステップで吐き出される deprecated のメッセージは、その対象となるモジュールが全て明るみになりますので、それらを書きだします。そこから検索エンジンを用いて最新のステータスを確認した上で、ベータ版でも躊躇せずモジュールのアップデートを図ります。あとは、次の対処方法の項に譲ります。
・対処方法:上述の方法で deprecated なモジュールのアップデートを図る場合、それは同じモジュールの最新メジャーバージョンだったりあるいは別のモジュールだったりすることもあります。また、依存関係の変更も迫られます。さらに、そうして変更された依存関係が既存のモジュールとコンフリクトする場合もあります。それらは、地味に対応策を探して適用するという「対策前進」を前提に対応します。そうしてワーニングの表出数に明らかな減少が見られた時点で、いったん作業を終えます。そして表示やパフォーマンスといった動作を確認した後、個人の判断でイシュー& PR を上げます。

今回の場合だと、corejs2, request, resolve-url, urix といったモジュール群は deprecated なもの、一方 @vue/server-renderer は依存関係に対応したもの、というふうに分類できます。とはいえ、個人の作業だと誤謬が起きがちなのは言うまでもありません。ですので、皆さんのご指摘には本当に助けられています。
あと、具体的な対象モジュールは diff でご確認ください、というふうに個人的にはしたいです。個人が自力で表現すると、抜け漏れや冗長さを招くリスクが避けられないためです。

ちなみに、私が deprecated を気にするのは、自身の職業を反映したものでもあります。
Deprecated な箇所は、vulnerability(脆弱性)となり得るという懸念があるためです。
オープンソースに古くから関わる者としては、いわゆる「本業」に触れることはあまりしません。
むしろ、避けるようにしています。
でも今回あえて「本業」を明かせば、普段は IT 監査やセキュリティ評価、現場への教育の実施、そして関連するプロジェクトを回したりしています。
その現場だと、私自身がここでやっているような報告を部下がしたなら即刻却下ですね(笑) それはまさに、 @goki90210 さんが指摘されているようなポイントがまるで抜けているためです。
でも、私はオープンソース・ソフトウェア・コミュニティは、敷居を下げる意味でそれっくらいラフでもいいんじゃないかと思っています

@goki90210
Copy link
Contributor

goki90210 commented Jul 6, 2020

具体的な対象モジュールは diff でご確認ください、というふうに個人的にはしたいです。

diffのみだと

  • yarn add/remove したのか package.jsonを直接編集したのかがわからない。

という(少なくとも私は)非常に重要だと考えている問題を抱えています。
(このような趣旨のコメントは@kaizumaki さんも上のコメントで言及されています。)

追加、削除、変更したモジュールは(少なくともpackage.jsonの範疇では)わかるはずですよね?
(yarn.lockは無理だと思います。)

このPRのように複数回のコミット/マージが入ると、何を行ったのかがdiffを追うだけでは非常に厳しくなります。

  1. core-js@3の導入(これは賛同は得られやすい)
  2. request, resolve-url, urixの問題の解消(※解決法は必ずしも1つとは限らない)
  3. Vue 3の導入(※ベータ版の導入の是非)

という大きく分けて3つの趣旨の異なる事象をまとめて実施しているので「ちょっと待った!」になってしまうのです。
(私は1. は賛成、2. は要議論、3. は反対です。)

@MaySoMusician MaySoMusician added the dependencies 依存性のあるファイルを更新するもの label Jul 6, 2020
@mcdmaster
Copy link
Contributor Author

@goki90210 いつも論理的でわかりやすいコメントをありがとうございます。
おっしゃるように、diff だけだと historical な change はとても追いにくいですね。
※不可能ではないにせよ、です。

皆さんは、どうしておられるんでしょう。
特に UI 系は、ちょっとした微修正がリリース間際まで頻繁に重ねられるケースが多いと思います。
そういう条件で、ヒストリを的確に捕捉するにはどうするか、あるいはどうすべきか(理想像、to-be)を示したリファレンス等があれば、ぜひ見てみたいです

@goki90210
Copy link
Contributor

結局core-jsも含めてもとに戻したのでしょうか?

@mcdmaster
Copy link
Contributor Author

@goki90210 お尋ねの主旨がわかりかねるのですけど、日中に行った作業は以下のとおりです。

  • たまたま、PR の中でチェックエラーが出ているのを発見した
  • 最新版(開発機)の package.json から、core-js3 が外れていることが判明した
  • ここで core-js3 に「戻す」のは、影響範囲が広くなると考えた
  • nuxt.config.tsbuild: babel: セクション内の core-js3 に関連する設定部分だけを「戻し」た上で、設定内容そのものは core-js2 用にした
  • モジュールの追加削除変更はなし

以上です。問題点がありましたらどうぞ。
この PR がリリースされる暁には、上記設定を core-js3 用に戻してやる必要があります。
備忘メモとして最初のコメントに書いておきましょうか。
あるいは何か、リリース時に heads-up できるアイディアがありましたらご共有をお願いします

@goki90210
Copy link
Contributor

@mcdmaster
理解しました。

@kaizumaki
Copy link
Collaborator

依存ライブラリのアップデートについては #4954 を出していただいていますが、こちらのPRとどちらを先に取り込んだほうがいいでしょうか...?
また、ただいま大規模開発案件が動いておりますので、ライブラリ関係のマージは来週になると思います。

@mcdmaster
Copy link
Contributor Author

@kaizumaki おそらく、 #4954 で出していただいているコンポーネント群の方が優先度は高いと思います。
そちらは、TS のバリデーションにも直に響きそうに見えますので…。
本 PR のリリース順序がその後でも、私はオーケーです。ご配慮ありがとうございます

@mcdmaster
Copy link
Contributor Author

mcdmaster commented Jul 15, 2020

いったん、現状をコミットしました。説明は、後付けでコメントします。
さらなる変更の可能性もあります。
為念、yarn install のときのスクショは以下になります。
コメント 2020-07-15 122107

[アップデートの説明]
今回のテーマは、大きく分けて以下の3点です。

  1. モジュールのメジャーアップグレードの実施
  2. Deprecated の解消
  3. 上記に伴う、設定ファイルの修正

まずは 1. から。対象は、以下のとおりです。

  • @nuxt/typescript-runtime: 0.4.10 -> 1.0.0
  • @nuxt/typescript-build: 0.6.7 -> 2.0.0
  • chokidar: 2.1.7 -> 3.4.0
  • core-js: 2.6.11 -> 3.6.5

次に、2. で対象とした deprecated なモジュール群は以下になります。

  • request
  • resolve-url
  • urix
  • chokidar 2.1.7
  • core-js 2.6.11

いずれもメンテナンスフリーズとなってしまったため、早期のアップグレードの実施によりセキュリティ・リスクの軽減を目指します。

最後に、3. についてです。
まず、core-js3 へのアップグレードに伴い、nuxt.config.ts に以下のブロックを追加しています。

    babel: {
      presets({ isServer }) {
        return [
          [
            '@nuxt/babel-preset-app',
            {
              buildTarget: isServer ? 'server' : 'client',
              corejs: {
                version: 3
              }
            }
          ]
        ]
      }
    }

詳細は、diff を参照してください。
また、依存関係のさらなる明確化と deprecated なモジュールとの着実な切り離しを図るため、package.json"resolutions" ブロックを作成しました。

  "resolutions": {
    "chokidar": "^3.4.0",
    "core-js": "^3.6.5",
    "css": "^3.0.0",
    "source-map-resolve": "^0.6.0"
  }

上記のように、"resolutions" ブロックには依存関係が明白なモジュール群の json レコードが記載されます。本 PR での対象は、以下の4つです。

  • chokidar
  • core-js
  • css
  • source-map-resolve

いずれも、前項の deprecated なモジュール群の呼出し元としての依存関係がありました。
これらも、本 PR を契機に最新版へのアップグレードを図っています。
アップグレードに伴い deprecated なモジュールの呼出しの中止が図られていたりと、改善が見られるためです

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies 依存性のあるファイルを更新するもの
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrading Essential Components
4 participants