-
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
perf: optimize local startup speed and add header Class configuration to drawer #4840
Conversation
|
WalkthroughThe pull request introduces several modifications across multiple files, primarily enhancing the configuration for VitePress and Vite applications. Key changes include the addition of SCSS preprocessing options and new properties in the drawer component for better styling. Additionally, package versions in the 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: 3
🧹 Outside diff range and nitpick comments (6)
internal/vite-config/src/config/application.ts (2)
89-89
: Performance enhancement: Additional paths for server warmupThe addition of
adapter
to the warmup paths will help optimize local startup speed by pre-warming these files. This aligns well with the PR's performance optimization goals.Consider monitoring the startup time impact. If it becomes too slow, you might want to be more selective about which files to pre-warm.
117-118
: LGTM: Enhanced SCSS processing configurationThe changes improve SCSS processing by:
- Using the modern Sass API
- Adding NodePackageImporter for better package resolution
This configuration enables more efficient SCSS processing and better package imports handling.
The modern API provides better performance, but ensure all your SCSS features are compatible with it. The NodePackageImporter might slightly impact build time but provides better maintainability for package imports.
docs/.vitepress/config/shared.mts (2)
62-64
: Consider optimizing PostCSS configurationWhile adding Sass optimization, consider also optimizing the PostCSS pipeline. The current setup with Tailwind and isolation could benefit from caching:
css: { postcss: { + postcssOptions: { + config: false, + }, plugins: [ tailwind(), postcssIsolateStyles({ includeFiles: [/vp-doc\.css/] }), ], },Also applies to: 70-73
Line range hint
62-105
: Consider chunking optimization for better startup performanceSince this PR focuses on startup performance, the build configuration could be further optimized:
build: { chunkSizeWarningLimit: Infinity, minify: 'terser', + rollupOptions: { + output: { + manualChunks: { + 'vue-libs': ['vue', '@vue/repl'], + 'markdown': ['markdown-it'], + } + } + } },This would help with:
- Better code splitting
- Improved caching
- Faster initial page loads
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (2)
135-141
: Consider extracting complex class binding to a computed property.While the implementation is correct, the template's readability could be improved by moving the complex class binding logic to a computed property.
Here's a suggested refactor:
+ const headerClasses = computed(() => + cn( + '!flex flex-row items-center justify-between border-b px-6 py-5', + headerClass, + { + 'px-4 py-3': closable, + }, + ) + ) <SheetHeader - :class="cn('!flex flex-row items-center justify-between border-b px-6 py-5', - headerClass, - { - 'px-4 py-3': closable, - }, - )" + :class="headerClasses" >
195-200
: Apply consistent pattern for footer class binding.For consistency with the header section, consider extracting the footer's class binding logic to a computed property as well.
Here's a suggested refactor:
+ const footerClasses = computed(() => + cn( + 'w-full flex-row items-center justify-end border-t p-2 px-3', + footerClass + ) + ) <SheetFooter v-if="showFooter" - :class="cn( - 'w-full flex-row items-center justify-end border-t p-2 px-3', - footerClass, - )" + :class="footerClasses" >
📜 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 (6)
docs/.vitepress/config/shared.mts
(2 hunks)internal/vite-config/src/config/application.ts
(3 hunks)internal/vite-config/src/plugins/vxe-table.ts
(0 hunks)packages/@core/ui-kit/popup-ui/src/drawer/drawer.ts
(2 hunks)packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue
(3 hunks)pnpm-workspace.yaml
(7 hunks)
💤 Files with no reviewable changes (1)
- internal/vite-config/src/plugins/vxe-table.ts
🔇 Additional comments (5)
internal/vite-config/src/config/application.ts (1)
9-9
: LGTM: Import statement aligns with SCSS configuration changes
The import of NodePackageImporter
is necessary for the enhanced SCSS processing configuration.
pnpm-workspace.yaml (3)
54-54
: LGTM: Synchronized package versions.
Good practice maintaining version parity between related packages:
- vee-validate and @vee-validate/zod both updated to ^4.14.7
Also applies to: 161-161
79-80
: Verify package updates impact on startup performance.
The updates appear to be minor version bumps or patches. However, since the PR's objective is to optimize local startup speed, please ensure these updates contribute to or at least don't negatively impact the performance goals.
#!/bin/bash
# Description: Check for performance-related changes in package usage
# Test: Look for performance-sensitive imports or configurations
echo "Checking for dynamic imports and lazy loading patterns..."
rg "import\((.*?)\)" --type ts --type vue
# Test: Search for any performance-related configurations
echo "Searching for build and performance configurations..."
fd "vite\.config\.(ts|js)$|\.env.*"
Also applies to: 93-93, 117-117, 120-120, 124-124, 137-137
177-178
: Verify the significant patch update for vxe-table.
The vxe-table package has been updated from ^4.8.2 to ^4.8.8, which includes multiple patch versions. Since this is a UI component library that could affect startup performance, please ensure these changes have been tested thoroughly.
✅ Verification successful
Based on the search results, I can now generate the final response since we have enough context about the vxe-table usage in the codebase:
The vxe-table update appears safe to proceed with.
The package is used in a modular way within the @vben/plugins
package, specifically in the packages/effects/plugins/src/vxe-table/
directory. The implementation is well-encapsulated with proper type definitions and initialization handling. Since this is a patch version update (4.8.2 to 4.8.8) and the package is used in a controlled manner through a plugin system, the update is unlikely to cause breaking changes or performance regressions.
Key observations:
- Usage is isolated to a dedicated plugin module
- Proper type definitions are maintained
- The update follows semantic versioning (patch version)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any breaking changes or performance improvements in the changelog
# Test: Search for vxe-table usage and potential breaking changes
echo "Searching for vxe-table usage in the codebase..."
rg "vxe-table" -A 5
# Test: Check if there are any configuration files specific to vxe-table
echo "Checking for vxe-table configuration files..."
fd "vxe.*\.(ts|js|json)$"
Length of output: 351
Script:
#!/bin/bash
# Let's try a different approach to find vxe-table usage
# Search for imports of vxe-table
echo "Searching for vxe-table imports..."
rg "from ['\"](vxe-table|vxe-pc-ui)" -A 2
# Search for VXETable component usage
echo "Searching for VXETable component usage..."
rg "VXETable" -A 2
# Search for package.json files to understand the dependency structure
echo "Checking package.json files..."
fd "package.json" --exec cat {}
Length of output: 48786
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (1)
59-60
: LGTM: Props addition aligns with requirements.
The addition of footerClass
and headerClass
props enhances the component's customization capabilities, aligning with the PR's objective to support header Class configuration.
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
footerClass
andheaderClass
properties.Bug Fixes
Chores