-
Notifications
You must be signed in to change notification settings - Fork 0
Add npm run up-deps
and update deps to latest (Vite 5=>6, Svelte, etc)
#150
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
Conversation
WalkthroughThis pull request updates the Changes
Suggested Reviewers
Possibly Related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (4)frontend/package.json (4)
The interactive mode might lead to inconsistent updates.
Platform-specific rollup dependency might affect builds on other platforms. ✅ Verification successfulPlatform-specific dependency is correctly configured The 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if other platform-specific rollup binaries are needed
rg "rollup.*gnu|rollup.*darwin|rollup.*windows" package.json
Length of output: 115 Script: #!/bin/bash
# Find package.json files
fd package.json
# Check platform-specific rollup deps in found package.json files
fd package.json --exec rg "rollup.*gnu|rollup.*darwin|rollup.*windows" {}
Length of output: 161
The Vite override might conflict with vitest's requirements. ✅ Verification successfulVite v6.0.5 is compatible with the setup Vitest 3.0.0-beta.2 doesn't specify a Vite peer dependency, and the current configuration in vite.config.ts works with Vite 6. The force optimization is already handled for git branch switches. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check vitest's vite peer dependency
curl -s https://registry.npmjs.org/vitest/3.0.0-beta.2 | jq -r '.peerDependencies.vite'
Length of output: 92 Script: #!/bin/bash
# Check package.json for vitest version and dependencies
cat frontend/package.json | jq -r '.dependencies.vitest, .devDependencies.vitest'
# Check if vitest is used in the project
rg "vitest" -l
Length of output: 266 Script: #!/bin/bash
# Check vite config
cat frontend/vite.config.ts
# Check vitest version requirements
curl -s https://registry.npmjs.org/vitest/3.0.0-beta.2 | jq -r '.peerDependencies'
Length of output: 1128
Risky: Multiple beta/major version updates Using beta versions (Svelte 5, vitest) in production is risky. Vite 6 needs migration validation. Also applies to: 55-56 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/package.json (1)
76-77
: Remove redundant Vite overrideVite is already specified in devDependencies with the same version. Having it in overrides is redundant unless there's a specific reason to force this version for transitive dependencies.
"overrides": { "cross-spawn": "^7.0.6", "cookie": "^0.7.0", "glob": "^9.0.0", - "vite": "^6.0.5" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
frontend/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
frontend/package.json
(3 hunks)
🔇 Additional comments (2)
frontend/package.json (2)
66-66
: LGTM: Safe patch update for dotenv
The update to dotenv is a patch version change that should not introduce any breaking changes.
24-55
:
Several major version updates introduce potential risks:
-
Vite 6 (line 54):
- Released recently with breaking changes
- Might have stability issues
- Requires Node.js 18+
-
Svelte 5 (line 46):
- Still in preview/beta
- Introduces Runes and breaking changes
- Migration guide should be followed
-
Vitest beta (line 55):
- Using beta version in production dependencies
Let's verify Node.js compatibility and potential breaking changes:
Recommendations:
- Add
"engines"
field to package.json to specify Node.js version requirements - Consider staying on Vite 5 until 6.x stabilizes
- Use stable version of Vitest instead of beta
"test:unit:once": "vitest --run", | ||
"up-deps": "npx npm-check-updates -i" |
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.
🛠️ Refactor suggestion
Consider using safer dependency update flags
The new up-deps
script uses the interactive mode (-i
) of npm-check-updates, which might lead to inconsistent updates. Consider using more specific flags for better control and reproducibility:
-"up-deps": "npx npm-check-updates -i"
+"up-deps": "npx npm-check-updates --target minor --reject svelte,@sveltejs/*,vite --format group"
This approach:
- Limits updates to minor versions only
- Excludes major framework updates that need careful migration
- Groups updates by type for better review
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"test:unit:once": "vitest --run", | |
"up-deps": "npx npm-check-updates -i" | |
"test:unit:once": "vitest --run", | |
"up-deps": "npx npm-check-updates --target minor --reject svelte,@sveltejs/*,vite --format group" |
ff182ab
to
896534d
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/package.json (1)
37-37
: Create follow-up issue for pending updatesAs mentioned in the PR objectives,
bits-ui
andsvelte-radix
were intentionally not updated pending visual validation.Would you like me to create a GitHub issue to track the visual validation and future updates of these packages?
Also applies to: 50-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
frontend/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
frontend/package.json
(3 hunks)
🔇 Additional comments (2)
frontend/package.json (2)
20-21
: Consider using safer dependency update flags
The new up-deps
script uses the interactive mode (-i
) of npm-check-updates, which might lead to inconsistent updates.
76-77
: Track temporary vite override
The vite override is added to handle vitest's internal dependencies still requiring vite@5. This should be temporary.
Let's check if there are any open issues or PRs addressing this:
Would you like me to create a GitHub issue to track the removal of this override once vitest updates its dependencies?
"postcss": "^8.4.49", | ||
"prettier": "^3.4.2", | ||
"prettier-plugin-svelte": "^3.3.2", | ||
"svelte": "^5.15.0", |
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.
💡 Codebase verification
Major version updates require attention
Based on the verification results:
-
Vite 6.0.0 is a stable release with mostly internal changes and improvements. The breaking changes are minimal and mainly affect:
- Node.js version requirements
- Some internal APIs and types
- Asset handling improvements
-
Svelte 5.15.0 is still in development and not recommended for production use. The official Svelte team marks version 5 as preview/beta.
-
Using beta versions of testing libraries (vitest) in production is not recommended.
Recommendations:
- Consider staying on Svelte 4.x for production use until version 5 is officially stable
- Wait for vitest to reach a stable 3.0.0 release
- Proceed with Vite 6 upgrade after thorough testing as its changes are mostly internal
🔗 Analysis chain
Review major version updates carefully
Several significant version updates require careful consideration:
- Svelte 5.x is still in beta/preview and might not be production-ready
- vitest 3.0.0-beta.2 is a beta version
- Vite 6.x is a major version update that might have breaking changes
Please ensure:
- Thorough testing of the application with these versions
- Review of migration guides for breaking changes
- Consider waiting for stable releases of beta packages
Let's check the migration guides and known issues:
Also applies to: 54-54, 55-55
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for migration guides and known issues
# Check Vite's releases for breaking changes
gh api repos/vitejs/vite/releases | jq -r '.[] | select(.tag_name=="v6.0.0") | .body'
# Check Svelte's releases for stability status
gh api repos/sveltejs/svelte/releases | jq -r '.[] | select(.tag_name | startswith("v5.")) | .body'
Length of output: 304
Script:
#!/bin/bash
# Let's check the changelog directly and look for breaking changes
curl -s https://raw.githubusercontent.com/vitejs/vite/v6.0.0/packages/vite/CHANGELOG.md | head -n 100
# Let's also check if Svelte 5 is marked as beta/preview in package.json
rg "svelte.*beta|svelte.*preview" -A 2 -B 2
# Check for any migration guides or breaking changes documentation
fd "MIGRAT|BREAKING|UPGRADE" -i
Length of output: 17326
npm run deps
and update deps to latest (Vite 5=>6, Svelte, etc)npm run up-deps
and update deps to latest (Vite 5=>6, Svelte, etc)
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.
nice stuff. all endpoints/new packages work, cicd is happy :)
896534d
to
fc58a4c
Compare
Summary
Due to some of our odd issues (out of memory, etc) I thought it would be helpful to be on the latest Vite and Svelte versions.
I also added
npm run up-deps
similar to pnpm PR but leveraging npm-check-updates package to make bumping versions easier (and interactive).This updates all dependencies except
bits-ui
(0.21.16 => 0.22.0) andsvelte-radix
(1.1.1 => 2.0.1) which are used by shadcn-svelte. They are probably safe to update (especially bits-ui) but would require more visual validation.I had to add a
vite
override as some internal deps ofvitest
were still requestingvite@5
(and oddly this isn't an issue when usingpnpm
as I have a similar vite 6 setup on other repos.Checklist
Summary by CodeRabbit
New Features
Updates