zenn-dev / zenn-editor

Convert markdown to html in Zenn format
MIT License
606 stars 76 forks source link

chore: fix pnpm version in package.json #475

Closed teppeis closed 8 months ago

teppeis commented 9 months ago

:bookmark_tabs: Summary

このリポジトリでcorepack環境下のpnpmを利用するとエラーになります。

ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your pnpm version is incompatible with "/Users/teppei_sato/src/github.com/zenn-dev/zenn-editor".

Expected version: >=8
Got: 7.19.0

https://github.com/zenn-dev/zenn-editor/pull/460 の修正漏れです。 engines"pnpm": ">=8"なのに、packageManagerではv7のままなので、corepackを使っているとエラーになりpnpmを利用不可能な状態になってます。

バージョンを更新する場合は、corepack upまたはcorepack installを実行してpackageManagerフィールドを更新する必要があります。

:clipboard: Tasks

プルリクエストを作成いただく際、お手数ですが以下の内容についてご確認をお願いします。

より詳しい内容は Pull Request Policy を参照してください。

cm-igarashi-ryosuke commented 8 months ago

@teppeis PRありがとうございます!

corepack 0.20.0 以降( corepack up または corepack install が追加されたバージョン)で問題が再現することを確認できました。

1点ご質問させていただきたいのですが。 corepackはNode.jsにバンドルされたものを使うことが多いと思います。corepackのバージョンはNode.jsのバージョンに依存しますので、このプロジェクトでもNode.jsの最低バージョンを指定したほうが良いでしょうか?

teppeis commented 8 months ago

@cm-igarashi-ryosuke corepack v0.19 以下でも発生しますよ。 例えば Node v18.18.2 にバンドルされている corepack v0.19.0 で corepack enable pnpm すると pnpm v7.19.0 がインストールされ、pnpm install等を実行すると同様のエラーになりました。

Nodeバージョンについては、今回の事案では関係ないと思います。

teppeis commented 8 months ago

別の修正方法として、packageManagerフィールド自体を削除してしまっても良いと思います。 特定のバージョンを指定することに強い意図がなければ、今後アップデートしていくのも面倒なので削除した方が楽かもしれません。

cm-igarashi-ryosuke commented 8 months ago

@teppeis

corepack v0.19 以下でも発生しますよ。

ありがとうございます。再現できました。わたしの環境ではグローバルインストールされたpnpmがあったので、 pnpm install 等ではそちらが優先され、エラーにならなかったようです。

別の修正方法として、packageManagerフィールド自体を削除してしまっても良いと思います。

たしかに、 packageManager フィールドがなくてもcorepackの使用に問題ないですね。 今のところpnpmのバージョン指定はメジャーバージョンレベルで十分ですので、 packageManager フィールドを削除することにしようと思います。 こちらで対応しますので、対応後にこのPRはクローズします。

Node.jsのバージョンについては、別で頂いていたIssue https://github.com/zenn-dev/zenn-editor/issues/476 の件もありますので、別途考えようと思います。

いろいろとアドバイスいただいて感謝です!

teppeis commented 8 months ago

了解です。ではこちらのPRはクローズします。