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

package-lock.json問題に終止符を打つためにyarnを採用する #70

Closed
wants to merge 7 commits into from

Conversation

yamachu
Copy link
Member

@yamachu yamachu commented Aug 11, 2021

ref: https://github.com/Hiroshiba/voicevox/issues/37

package-lock.jsonが頻繁に変わってしまうことの原因として、
npmのversionの差異が生じている状態で環境構築時にnpm installを実行してしまうことが挙げられる。
対策として npm ci を使うことを強制させるという手も考えられるが、npm install をしてしまうことも大いに有り得る。

バージョンの差異はengineフィールドで吸収することも可能だが
https://github.com/Hiroshiba/voicevox/pull/50

グローバルインストールしたnpmなどを利用していて、それをupgradeした場合にバージョンを落とすことが困難である。

そのため、それらのコントロールしづらい状況に対応すべくパッケージ管理に yarn を導入する。

package-lock.json からの migrate は以下のコマンドで実行した

 yarn import
yarn import v1.22.5
info found npm package-lock.json, converting to yarn.lock
warning [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
warning @vue/cli-plugin-eslint > [email protected]: This loader has been deprecated. Please use eslint-webpack-plugin
warning @vue/cli-service > [email protected]: 3.x is no longer supported
warning @vue/cli-plugin-e2e-cypress > cypress > [email protected]: Debug versions >=3.2.0 <3.2.7 || >=4 <4.3.1 have a low-severity ReDos regression when used in a Node.js environment. It is recommended you upgrade to 3.2.7 or 4.3.1. (https://github.com/visionmedia/debug/issues/797)
warning @vue/cli-service > webpack-dev-server > [email protected]: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
warning electron > extract-zip > [email protected]: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
warning @vue/cli-plugin-unit-mocha > mocha > [email protected]: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
warning @vue/cli-plugin-e2e-cypress > cypress > [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
warning @vue/cli-service > @types/webpack > @types/[email protected]: This is a stub types definition. anymatch provides its own type definitions, so you do not need this installed.
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > @hapi/[email protected]: Switch to 'npm install joi'
warning @vue/cli-plugin-unit-mocha > jsdom > [email protected]: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142
warning @vue/cli-service > webpack-dev-server > chokidar > [email protected]: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
warning @vue/cli-plugin-unit-mocha > mochapack > @babel/runtime-corejs2 > [email protected]: core-js@<3.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Please, upgrade your dependencies to the actual version of core-js.
warning @vue/cli-plugin-e2e-cypress > cypress > request > [email protected]: this library is no longer supported
warning @vue/cli-plugin-e2e-cypress > cypress > url > [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > @hapi/joi > @hapi/[email protected]: Moved to 'npm install @sideway/address'
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > @hapi/joi > @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > @hapi/joi > @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
warning @vue/cli-plugin-babel > @vue/cli-shared-utils > @hapi/joi > @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
warning Import of "bindings@^1.5.0" for "voicevox > @vue/cli-service > webpack-dev-server > chokidar > fsevents" failed, resolving normally.
warning Import of "nan@^2.12.1" for "voicevox > @vue/cli-service > webpack-dev-server > chokidar > fsevents" failed, resolving normally.
warning Import of "slice-ansi@^1.0.0" for "voicevox > vue-cli-plugin-electron-builder > electron-builder > dmg-builder > dmg-license > cli-truncate" failed, resolving normally.
warning Import of "string-width@^2.0.0" for "voicevox > vue-cli-plugin-electron-builder > electron-builder > dmg-builder > dmg-license > cli-truncate" failed, resolving normally.
warning Import of "cli-truncate@^1.1.0" for "voicevox > vue-cli-plugin-electron-builder > electron-builder > dmg-builder > dmg-license > iconv-corefoundation" failed, resolving normally.
warning Import of "node-addon-api@^1.6.3" for "voicevox > vue-cli-plugin-electron-builder > electron-builder > dmg-builder > dmg-license > iconv-corefoundation" failed, resolving normally.
warning @vue/cli-plugin-typescript > fork-ts-checker-webpack-plugin > micromatch > snapdragon > source-map-resolve > [email protected]: https://github.com/lydell/resolve-url#deprecated
warning @vue/cli-plugin-typescript > fork-ts-checker-webpack-plugin > micromatch > snapdragon > source-map-resolve > [email protected]: Please see https://github.com/lydell/urix#deprecated
warning " > [email protected]" has unmet peer dependency "webpack@^4.36.0 || ^5.0.0".
warning "@openapitools/openapi-generator-cli > @nestjs/[email protected]" has incorrect peer dependency "rxjs@^6.0.0".
warning "@openapitools/openapi-generator-cli > @nestjs/[email protected]" has incorrect peer dependency "rxjs@^6.0.0".
warning "@vue/cli-plugin-unit-mocha > [email protected]" has unmet peer dependency "webpack@^4.0.0".
success Saved lockfile.
✨  Done in 24.32s.

yarnに変更するに辺り残タスクとして

README.md Outdated Show resolved Hide resolved
Comment on lines -23 to -28
- uses: actions/cache@master
with:
path: ~/.npm
key: ${{ env.cache-version }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ env.cache-version }}-node-
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packge-lock.json を監視するキャッシュではなくなるのでrm
またこういった記述をしなくても setup-node で npm や yarn のパッケージのキャッシュを行える機構があるのでそれに乗っかる

restore-keys: |
${{ env.cache-version }}-${{ runner.os }}--electron-builder-cache-
- run: npm ci
- run: yarn install --frozen-lockfile
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm ci と等価ではないけれども、もしも package.json で使っているパッケージが yarn.lock にかかれていない場合差分が出ていると検知し、エラーで停止するようになります
https://classic.yarnpkg.com/en/docs/cli/install/#toc-yarn-install-frozen-lockfile

@Hiroshiba
Copy link
Member

プルリクエストありがとうございます。
yarnが初学者向けにも難しくないものか見極めたいので、レビューまでしばらく時間がかかりそうです。

@Hiroshiba
Copy link
Member

他の方の開発環境に影響があるので、なんとなくですがまたアップデート直前に検証&方針決定したいと思います。

@Hiroshiba
Copy link
Member

yarnを使わない今までの手法で、初学者の方も環境構築ができなくはないかもしれません。
https://ch.dlsite.com/matome/138914 (VOICEVOXにコミットしてくださった方の体験ブログ記事です)

なのでいまのところnpmで十分であるという判断をしたいと思います。

しかし、初学者の方がnpm installした際にpackage-lock.jsonが問題になるかもしれません。
そのときにまたyarnを検討する、というのはどうでしょう。

その場合、プルリクエストはcloseしていただいても、openのままであってもどちらでも大丈夫です。

@yamachu
Copy link
Member Author

yamachu commented Aug 22, 2021

ご検討ありがとうございました。

  • READMEにセットアップは npm ci で行うようにアナウンスし、開発に参加している方がそれに倣ってセットアップを行えるようになった
  • lock ファイルのフォーマットが変わる v7 系ではなく、v6 系を使うように engines で強制するようになった
  • 開発初期で機能追加のために npm install をすることが多かったが、それも減ってきた

みたいな当初解決したかった package-lock.json が書き換わる要因が減ってきたこともあり、今無理して yarn を入れる必要はなくなったようにも思えます。

継続的に使用パッケージの脆弱性対策や、その他機能導入のためのバージョンアップなどを行う際に lock 周りで問題が起こり始めたときにまた考え始めるでいいのかもしれません。

現時点このPRは不要に思えるので Close いたします 🙇

@yamachu yamachu closed this Aug 22, 2021
@yamachu yamachu deleted the hello-yarn branch August 22, 2021 08:04
@Hiroshiba
Copy link
Member

ありがとうございます。
仰るとおり、また問題が起こり始めたタイミングで起用を考えたいと思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants