-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
refactor: upgrade unbuild to 3.0.0 rc version #4797
Conversation
|
WalkthroughThe changes in this pull request involve modifications to configuration files related to the Tailwind CSS and Vite build processes. Key updates include the removal of Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Outside diff range and nitpick comments (3)
turbo.json (1)
19-24
: Document the purpose of VitePress outputsConsider adding a comment in the configuration file to explain why these VitePress outputs are being tracked, especially since they seem unrelated to the unbuild upgrade.
"build": { "dependsOn": ["^build"], + // Tracks both regular build outputs and VitePress documentation builds "outputs": [ "dist/**", "dist.zip", ".vitepress/dist.zip", ".vitepress/dist/**" ] },
internal/tailwind-config/package.json (1)
51-51
: LGTM! Consider version pinning for better stability.The replacement of
@vben/node-utils
with@manypkg/get-packages
is consistent with the refactoring goals. The "catalog:" version specifier follows the project convention.However, for better stability, consider pinning the version to a specific release, especially since this is a core build dependency.
- "@manypkg/get-packages": "catalog:", + "@manypkg/get-packages": "^3.0.0",Also applies to: 64-64
internal/tailwind-config/src/index.ts (1)
Line range hint
17-22
: Consider adding error handling for package resolution.The package resolution logic could benefit from error handling to gracefully handle cases where packages cannot be found or accessed.
const tailwindPackages: string[] = []; -packages.forEach((pkg) => { +try { + packages.forEach((pkg) => { // apps目录下和 @vben-core/tailwind-ui 包需要使用到 tailwindcss ui // if (fs.existsSync(path.join(pkg.dir, 'tailwind.config.mjs'))) { - tailwindPackages.push(pkg.dir); + if (pkg.dir) { + tailwindPackages.push(pkg.dir); + } // } -}); + }); +} catch (error) { + console.error('Failed to resolve package directories:', error); + // Fallback to a default set of directories or throw if critical + throw error; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
internal/tailwind-config/build.config.ts
(0 hunks)internal/tailwind-config/package.json
(3 hunks)internal/tailwind-config/src/index.ts
(1 hunks)internal/vite-config/src/config/common.ts
(1 hunks)pnpm-workspace.yaml
(1 hunks)scripts/turbo-run/src/run.ts
(0 hunks)turbo.json
(1 hunks)
💤 Files with no reviewable changes (2)
- internal/tailwind-config/build.config.ts
- scripts/turbo-run/src/run.ts
✅ Files skipped from review due to trivial changes (1)
- pnpm-workspace.yaml
🔇 Additional comments (4)
internal/vite-config/src/config/common.ts (1)
6-6
: Acceptable increase in chunk size warning limit, but verify if needed.
The increase to 2000KB is reasonable for modern web applications. However, since this change was made alongside the unbuild upgrade:
- Verify if the new unbuild version is actually producing larger chunks that warrant this increase
- Consider monitoring your bundle sizes to ensure they stay within acceptable limits for your target users
✅ Verification successful
Let me gather more information about the build configuration and chunk sizes.
Let me check the build configurations to understand if there are any size-related settings or if the unbuild upgrade is causing larger chunks.
Based on the gathered information, I can now provide a final response:
Increase in chunk size warning limit is justified with unbuild upgrade
The change from 1000KB to 2000KB coincides with a major version upgrade of unbuild from v2.0.0 to v3.0.0-rc.11. The git history shows this was part of the same upgrade PR, suggesting the increase was needed to accommodate changes in how unbuild 3.0 handles chunking and bundling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the current chunk sizes to verify if the increase is necessary
# Look for build output size information in recent build logs
rg -A 5 "chunk.*size.*KB"
# Search for any existing size-related configurations
rg -g '*.{js,ts,json}' -A 3 'chunkSize|bundleSize|maxSize'
Length of output: 357
Script:
#!/bin/bash
# Check git history for previous value of chunkSizeWarningLimit
git log -p -S "chunkSizeWarningLimit" internal/vite-config/src/config/common.ts
# Look for build-related files and configurations
fd -e json -e js -e ts build.config
Length of output: 1499
Script:
#!/bin/bash
# Check build configurations for size-related settings
cat internal/vite-config/build.config.ts
# Look for unbuild-related changes in recent commits
git log -p --since="1 month ago" | rg -A 5 -B 5 "unbuild"
# Check if there are any existing large chunks in the codebase
rg -g '*.{js,ts}' -l "\.chunk\."
Length of output: 14156
turbo.json (1)
19-24
: Verify VitePress build outputs exist
The addition of VitePress-related outputs suggests documentation is being generated, but we should verify these paths are actually being used.
✅ Verification successful
Let me gather more information to verify if VitePress is actually being used in the project.
VitePress outputs in turbo.json are valid and necessary
The VitePress-related outputs in turbo.json are correct and justified:
- Found VitePress configuration in
@vben/docs
package with proper dependencies and build scripts - Extensive documentation structure present in
docs/src/
with numerous markdown files - The package uses VitePress for documentation building (confirmed by
"build": "vitepress build"
script)
The .vitepress/dist/**
and .vitepress/dist.zip
outputs are valid cache targets for the documentation build process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if VitePress is configured and these paths are valid
# Look for VitePress configuration
echo "Checking for VitePress configuration..."
fd -t f "config\.(js|ts)" -E "node_modules" | rg -l "vitepress"
# Look for references to these output paths
echo "Checking for references to VitePress dist paths..."
rg -l "\.vitepress/dist"
Length of output: 299
Script:
#!/bin/bash
# Check for VitePress dependencies and configuration
# Look for VitePress in package.json files
echo "Checking for VitePress in package dependencies..."
fd package.json -E "node_modules" -x cat {} | jq -r 'select(.dependencies.vitepress or .devDependencies.vitepress)'
# Look for .vitepress directory
echo "Checking for .vitepress directory..."
fd -t d "\.vitepress$"
# Look for markdown files that might be used with VitePress
echo "Checking for markdown files in potential docs directory..."
fd -t f "\.md$" -E "node_modules" -E "README.md" -E "CHANGELOG.md"
Length of output: 4386
internal/tailwind-config/src/index.ts (2)
14-14
: Verify package resolution behavior.
The addition of process.cwd()
makes the working directory explicit, which is good. However, we should verify that this change maintains the expected package resolution behavior, especially for monorepo setups.
#!/bin/bash
# Description: Verify package resolution behavior
# Test: Check if there are other instances of getPackagesSync to ensure consistent usage
rg "getPackagesSync\(" --type ts
# Test: Check if there are workspace configuration files that might affect package resolution
fd -g "pnpm-workspace.yaml" -g "lerna.json" -g "rush.json"
6-6
: Verify the @manypkg/get-packages dependency.
The change from @vben/node-utils
to @manypkg/get-packages
looks good, but let's ensure the dependency is properly declared.
✅ Verification successful
@manypkg/get-packages dependency is properly configured
The dependency is correctly declared in internal/tailwind-config/package.json
using the workspace catalog reference ("catalog:"). The package is also properly referenced in the monorepo's workspace configuration (pnpm-workspace.yaml).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify @manypkg/get-packages dependency declaration
# Test: Check if @manypkg/get-packages is declared in package.json
rg -l "@manypkg/get-packages" "package.json"
# Test: Check the version constraint
rg '"@manypkg/get-packages":\s*"[^"]*"' "package.json"
Length of output: 399
Script:
#!/bin/bash
# Let's check package.json files in the monorepo structure
fd package.json --type f --exec grep -l "@manypkg/get-packages" {}
# Check if pnpm-workspace.yaml exists and its content
fd pnpm-workspace.yaml --type f --exec cat {}
# Check the content of internal/tailwind-config/package.json specifically
cat internal/tailwind-config/package.json
# Check if @vben/node-utils is still being used
rg "@vben/node-utils" --type ts
Length of output: 8148
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Dependencies
unbuild
to^3.0.0-rc.11
.@manypkg/get-packages
and removed@vben/node-utils
from package configurations.Configuration Updates
1000
to2000
.