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

chore: yarn ではなく pnpm を使うように変更 #4186

Merged
merged 8 commits into from
Feb 16, 2024

Conversation

Tokky0425
Copy link
Member

@Tokky0425 Tokky0425 commented Jan 15, 2024

Related URL

Overview

T/O

What I did

corepack 対応は別プルリクでやります!

Capture

@Tokky0425 Tokky0425 changed the title Switch to pnpm chore: Switch to pnpm Jan 15, 2024
Copy link

netlify bot commented Jan 15, 2024

Deploy Preview for smarthr-ui ready!

Name Link
🔨 Latest commit 548a539
🔍 Latest deploy log https://app.netlify.com/sites/smarthr-ui/deploys/65b85d744a0e320008cb23aa
😎 Deploy Preview https://deploy-preview-4186--smarthr-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Tokky0425 Tokky0425 force-pushed the switch-to-pnpm branch 3 times, most recently from 05d2065 to f84adae Compare January 16, 2024 10:57
@Tokky0425 Tokky0425 force-pushed the switch-to-pnpm branch 2 times, most recently from 548a539 to fdbe9d7 Compare January 30, 2024 02:41
@Tokky0425
Copy link
Member Author

Netlify が爆破されたら rebase してプルリクオープンします

@Tokky0425 Tokky0425 self-assigned this Feb 7, 2024
@Tokky0425 Tokky0425 changed the title chore: Switch to pnpm chore: yarn ではなく pnpm を使うように変更 Feb 15, 2024
@Tokky0425 Tokky0425 marked this pull request as ready for review February 15, 2024 02:55
@Tokky0425 Tokky0425 requested a review from a team as a code owner February 15, 2024 02:55
@Tokky0425 Tokky0425 requested review from AtsushiM and uknmr and removed request for a team February 15, 2024 02:55
check-source:
steps:
- run: yarn audit --groups dependencies
- run: yarn lint
- run: pnpm audit --prod
Copy link
Member Author

@Tokky0425 Tokky0425 Feb 15, 2024

Choose a reason for hiding this comment

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

yarn audit --groups dependencies と同等のものに変更しています。
https://pnpm.io/ja/cli/audit#--prod--p

key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ runner.os }}-yarn-
cache: 'pnpm'
Copy link
Member Author

Choose a reason for hiding this comment

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

キャッシュに関してはこれだけで良さそうでした。
https://pnpm.io/ja/continuous-integration#github-actions

Comment on lines +179 to +186
"pnpm": {
"overrides": {
"@babel/helper-compilation-targets": "^7.23.6",
"@types/react": "^18.2.55",
"minimist": "1.2.8",
"react": "^18.2.0",
"react-dom": "^18.2.0"
}
Copy link
Member Author

@Tokky0425 Tokky0425 Feb 15, 2024

Choose a reason for hiding this comment

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

yarn の resolutions と同等の記述になります。
https://pnpm.io/ja/package_json#pnpmoverrides

@@ -1,6 +1,6 @@
{
"extends": ["github>kufu/renovate-config"],
"npm": {
"postUpdateOptions": ["yarnDedupeHighest"]
"postUpdateOptions": ["pnpmDedupe"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +4 to +8
"compilerOptions": {
// stories.tsx で発生する下記のエラーを避けるため、tsconfig.json ではなくこちらで `"declaration": true` を指定
// https://github.com/microsoft/TypeScript/issues/47663#issuecomment-1519138189
"declaration": true,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

下記の条件に当てはまる変数部分でエラーが発生してしまっていました。(エラーが発生していたのは .stories.tsx のファイル)

  • tsconfig.json"declaration": true となっている
  • infer を使った型推論がされている
  • export されている

これは、 yarn から pnpm に切り替えたことで、 node_modules の管理方式が hoisted から isolated によって発生したものです。
詳しい説明は下記の issue に書いてあります。
microsoft/TypeScript#47663 (comment)

"declaration": true はビルド時に型ファイルを吐き出すかどうかの設定ですが、storybook が参照する tsconfig.json でこれが true になっている必要はないので、tsconfig.build.json 側でその指定をするように変更しました。

@Tokky0425 Tokky0425 enabled auto-merge (squash) February 15, 2024 10:22
Copy link
Collaborator

@uknmr uknmr left a comment

Choose a reason for hiding this comment

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

pnpm install して pnpm dev で問題なく動作することを確認しました!

Copy link
Member

@AtsushiM AtsushiM left a comment

Choose a reason for hiding this comment

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

👍

@uknmr
Copy link
Collaborator

uknmr commented Feb 16, 2024

あぁ、何らかの auto-merge がされてしまったのか yarn.lock がコンフリクトしてしまった:sob:

@Tokky0425 Tokky0425 merged commit c489824 into master Feb 16, 2024
8 checks passed
@Tokky0425 Tokky0425 deleted the switch-to-pnpm branch February 16, 2024 04:03
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.

3 participants