-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
fix: remove vite-plugin-lib-inject-css #4793
Conversation
|
WalkthroughThis pull request introduces several changes across multiple files, primarily focusing on the removal of 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: 0
🧹 Outside diff range and nitpick comments (2)
packages/@core/base/design/package.json (1)
31-31
: Resolve inconsistency between runtime and publish exportsThere's an inconsistency between the runtime exports (pointing to CSS) and publish exports (pointing to JS). This dual configuration could lead to confusion and potential issues:
- Runtime:
"default": "./dist/style.css"
- Publish:
"default": "./dist/index.mjs"
Consider:
- Aligning both configurations to use the same export
- Documenting the reasoning behind this dual configuration if it's intentional
- Providing clear migration guidelines for package consumers
Also applies to: 34-40
internal/vite-config/src/plugins/index.ts (1)
226-235
: Clean plugin architecture maintained.The plugin system's modular design made this removal straightforward. The conditional plugin loading pattern continues to provide good separation of concerns and flexibility.
Consider documenting this plugin system architecture in the project's technical documentation, as it provides a great example of extensible plugin management.
📜 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)
apps/backend-mock/nitro.config.ts
(1 hunks)internal/vite-config/package.json
(0 hunks)internal/vite-config/src/config/library.ts
(0 hunks)internal/vite-config/src/plugins/index.ts
(1 hunks)internal/vite-config/src/typing.ts
(0 hunks)packages/@core/base/design/package.json
(1 hunks)pnpm-workspace.yaml
(0 hunks)
💤 Files with no reviewable changes (4)
- internal/vite-config/package.json
- internal/vite-config/src/config/library.ts
- internal/vite-config/src/typing.ts
- pnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (1)
- apps/backend-mock/nitro.config.ts
🔇 Additional comments (2)
packages/@core/base/design/package.json (1)
31-31
:
This change modifies the default export from a JavaScript module to a CSS file, which is a breaking change that could affect package consumers.
Let's verify the impact of this change:
✅ Verification successful
✅ Default export change to CSS is intentional and safe
Based on the documentation and usage patterns found:
- The package is primarily used for providing styles, as shown in the documentation
- Current imports are simple style imports (
import '@vben-core/design'
) - The package maintains backward compatibility through
publishConfig
- This change aligns with the documented purpose of the package for styling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for direct imports of @vben-core/design to identify affected code
rg -l '@vben-core/design['"'"'"]' --type ts --type vue
# Check if there are any style imports that might conflict
rg -l '@vben-core/design/.*style' --type ts --type vue
Length of output: 168
Script:
#!/bin/bash
# Search for direct imports using ripgrep without file type restrictions
rg -l '@vben-core/design'
# Check package.json to understand the full export configuration
cat "packages/@core/base/design/package.json"
# Look for any documentation about this package's usage
fd "README" packages/@core/base/design
Length of output: 1344
Script:
#!/bin/bash
# Check the content of files that import this package to understand usage patterns
cat "packages/styles/src/index.ts"
cat "packages/styles/README.md"
cat "docs/src/guide/essentials/styles.md"
# Look for any migration guides or documentation about this change
rg -l "vite-plugin-lib-inject-css"
Length of output: 2266
internal/vite-config/src/plugins/index.ts (1)
227-227
: LGTM! Verify documentation and migration steps.
The removal of injectLibCss
from options destructuring is clean and aligns with the PR objective to remove vite-plugin-lib-inject-css.
Let's verify the documentation and potential breaking changes:
✅ Verification successful
Let me gather more information about the plugin removal by checking the package dependencies and configuration files.
Clean removal of vite-plugin-lib-inject-css confirmed
The removal of injectLibCss
option is complete and clean. No remaining references were found in:
- Package dependencies
- Configuration files
- TypeScript interfaces
- Documentation (only general CSS-related content found)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining references to the removed plugin in docs and configs
echo "Checking for remaining references to vite-plugin-lib-inject-css..."
rg -i "vite-plugin-lib-inject-css|injectLibCss"
# Look for potential migration guides or documentation that needs updating
echo "Checking documentation files..."
fd -e md -e mdx | xargs rg -i "css.*(inject|plugin)"
Length of output: 1059
Script:
#!/bin/bash
# Check package.json for any remaining dependencies
echo "Checking package.json files..."
fd package.json | xargs rg -i "vite-plugin-lib-inject-css"
# Check vite config files for any remaining configurations
echo "Checking vite config files..."
fd -e ts -e js | xargs rg -i "injectLibCss|libInjectCss" -A 2 -B 2
# Check for any TypeScript interfaces or types related to the removed option
echo "Checking TypeScript definitions..."
ast-grep --pattern 'interface $_ {
$$$
injectLibCss?: $_
$$$
}'
Length of output: 418
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
New Features
@vben-core/design
package to point to the CSS file, making it the main entry point.Bug Fixes
vite-plugin-lib-inject-css
dependency from various configurations, streamlining the build process.Documentation
pnpm-workspace.yaml
to reflect the removal of thevite-plugin-lib-inject-css
entry from the catalog.