-
-
Notifications
You must be signed in to change notification settings - Fork 204
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
Switch to pnpm + vitest (away from yarn @ 1 and jest) #2196
Conversation
dca0bed
to
96ef8ae
Compare
eh update lockfile eh eh
96ef8ae
to
55571e2
Compare
@@ -119,7 +103,9 @@ | |||
"release-it": "^17.0.0", | |||
"sort-package-json": "^2.6.0", | |||
"typescript": "^5.2.2", | |||
"typescript-eslint": "^8.7.0" | |||
"typescript-eslint": "^8.7.0", | |||
"vite": "^5.4.9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NullVoxPopuli is this intentionally added? Why would we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, we import from vite in the vitest.config
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: pnpm/action-setup@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NullVoxPopuli can/should we explicitly set the version? just thinking that it won't drift some day to v10 once it released plus it's explicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it uses the packageManager
field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh cool! TIL
"engines": { | ||
"node": "18.* || 20.* || >= 21" | ||
}, | ||
"volta": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NullVoxPopuli maybe e could keep volta config but pin pnpm using it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure thing
pnpm is a strict package manager, and yarn can't be trusted.
Some tooling we want to use doesn't support yarn (release-plan, fixturify-project / scenario-tester, etc).
I needed to include a vitest switch in here, because I needed to debug some failures when I switch to pnpm, and jest in just bad at debugging (no integration with the debug terminal in vscode)
It's way faster to switch to vitest (done in < 1 minute) than to fix jest.
Additionally, it looks like there was a failing situation with the no-get rule. So that's been fixed here: 55571e2