fix: remove unused dependencies and tidy project #30043
Conversation
|
|
WalkthroughThe pull request introduces significant updates to various configuration files, primarily focusing on the Changes
Possibly related PRs
Suggested labels
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 (
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/vite/src/vite-node.ts (1)
74-80: Consider tracking this TODO in the issue tracker.The TODO comment indicates a dependency on Nitropack or h3 updates. To ensure this doesn't get lost, it would be valuable to track this in an issue.
Would you like me to help create a GitHub issue to track this technical debt item? The issue would include:
- Current workaround using viteNodePlugin
- Dependency on Nitropack/h3 middleware capabilities
- Steps needed to implement the preferred solution
packages/kit/src/template.ts (1)
Line range hint
89-93: LGTM! Consider adding a comment about the hash function usage.The template normalization logic remains sound with the new hash function. However, it might be helpful to add a comment explaining why we hash the source path.
if (!template.filename) { const srcPath = parse(template.src) + // Hash the source path to generate a unique filename while maintaining consistency across builds template.filename = (template as any).fileName || `${basename(srcPath.dir)}.${srcPath.name}.${hash(template.src)}${srcPath.ext}` }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
knip.json(1 hunks)package.json(2 hunks)packages/kit/package.json(1 hunks)packages/kit/src/template.ts(1 hunks)packages/nuxt/package.json(0 hunks)packages/rspack/package.json(0 hunks)packages/schema/build.config.ts(0 hunks)packages/schema/package.json(0 hunks)packages/ui-templates/package.json(2 hunks)packages/vite/package.json(0 hunks)packages/vite/src/vite-node.ts(3 hunks)packages/webpack/package.json(0 hunks)test/fixtures/basic/package.json(1 hunks)
💤 Files with no reviewable changes (6)
- packages/nuxt/package.json
- packages/rspack/package.json
- packages/schema/build.config.ts
- packages/schema/package.json
- packages/vite/package.json
- packages/webpack/package.json
✅ Files skipped from review due to trivial changes (1)
- packages/ui-templates/package.json
🔇 Additional comments (14)
test/fixtures/basic/package.json (2)
14-17: LGTM: Version pattern is consistent
The use of "latest" version is appropriate for test fixtures and maintains consistency with other devDependencies in this file.
14-14: Verify if 'defu' dependency is necessary
Given that the PR's goal is to remove unused dependencies, please verify that defu is actually required in this test fixture.
✅ Verification successful
'defu' dependency is correctly used and required
The defu package is actively used in the test fixture's nuxt.config.ts for merging runtime configurations, which is a common Nuxt.js pattern. The dependency is necessary for the test fixture to function properly.
- Used in
test/fixtures/basic/nuxt.config.tsto merge runtime configurations:nuxt.options.runtimeConfig = defu(nuxt.options.runtimeConfig, { public: { testConfig: 123, }, })
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for defu usage in the test fixture
rg -l "defu" "test/fixtures/basic"
# Check if defu is used in any test files referencing this fixture
rg -l "fixture-basic" "test" | xargs rg "defu"
Length of output: 198
Script:
#!/bin/bash
# Check how defu is used in nuxt.config.ts
cat test/fixtures/basic/nuxt.config.ts
# Look for defu usage patterns in test files
rg "defu" -A 3 -B 3 "test"
Length of output: 9016
knip.json (5)
13-16: LGTM! Good separation of concerns
Moving test-related entries to their own workspace section while keeping scripts in the root workspace improves the configuration structure.
17-22: LGTM! Well-structured test fixtures configuration
The patterns effectively cover all necessary test fixture files with efficient use of brace expansion for file extensions.
29-38: LGTM! Consistent with general package patterns
The changes maintain consistency with the general package patterns while preserving the specific structure needed for the Nuxt package.
23-28: Verify the impact of including additional file extensions
The pattern now includes .js and .mjs files in addition to .ts. While this provides more flexibility, we should verify that this doesn't accidentally include any test or mock files.
✅ Verification successful
Including .js and .mjs extensions in runtime pattern is safe
The runtime directory contains only legitimate runtime files (.mjs) with no test or mock files. The single occurrence of the word "stub" is in a comment about a workaround and not related to test files. The change to include .js and .mjs extensions is appropriate for the codebase structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potentially unwanted files in runtime directories
echo "Checking for test/mock files in runtime directories..."
fd -e js -e mjs -e ts . packages/*/src/runtime/ | grep -iE '(test|spec|mock|stub)'
Length of output: 206
Script:
#!/bin/bash
# Let's first check what files actually exist in runtime directories
echo "Listing all files in runtime directories:"
fd . packages/*/src/runtime/
# Then check the content of these files for any test-related code
echo -e "\nChecking file contents for test-related patterns:"
rg -i "test|spec|mock|stub" packages/*/src/runtime/
Length of output: 501
39-57: Verify the cross-package reference in rspack configuration
The rspack workspace references webpack's source files ("../webpack/src/index.ts"). While this might be intentional, we should verify this dependency relationship is documented and necessary.
✅ Verification successful
Cross-package reference in rspack is intentional and well-structured
The cross-package reference in the rspack configuration is intentional and properly implemented:
- The rspack package is designed to share configuration and source code with the webpack package, as evidenced by its build configuration
- It explicitly imports webpack's build config and extends it
- The entry points in knip.json correctly reflect this architectural decision where rspack reuses webpack's source files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for documentation of rspack-webpack relationship
echo "Checking for documentation of rspack-webpack relationship..."
fd "README.md" packages/rspack/ -x grep -l "webpack"
echo "Checking for webpack imports in rspack..."
rg -l "(?:require|import).*['\"]\.\.\/webpack" packages/rspack/
Length of output: 474
Script:
#!/bin/bash
# Let's try a different approach to verify the relationship
# Check package.json dependencies
echo "Checking package.json in rspack for webpack dependencies..."
fd "package.json" packages/rspack/ -x cat {} \; | jq '.dependencies,.devDependencies'
# Check for webpack imports with fixed regex
echo "Checking for webpack imports in rspack (fixed regex)..."
rg -l "from ['\"]../webpack" packages/rspack/
# Check for webpack path references
echo "Checking for webpack path references in rspack..."
rg -l "../webpack" packages/rspack/
# Look for documentation mentioning webpack
echo "Checking documentation for webpack mentions..."
rg -i "webpack" packages/rspack/ --type md --type txt
Length of output: 2223
Script:
#!/bin/bash
# Let's examine the specific files that showed webpack references
echo "Checking build.config.ts content..."
cat packages/rspack/build.config.ts
echo -e "\nChecking package.json content for webpack references..."
cat packages/rspack/package.json | grep -A 1 -B 1 "webpack"
echo -e "\nChecking if rspack is using webpack's source files..."
rg -p "from.*webpack/src" packages/rspack/
Length of output: 1459
packages/kit/package.json (2)
Line range hint 1-67: LGTM! Package structure and dependencies are well-maintained
The package.json structure is clean with:
- Appropriate use of workspace dependencies for internal packages
- Specific versions for external dependencies
- Clear exports configuration
40-40: Verify the ohash dependency addition and hash-sum removal
The addition of ohash and removal of hash-sum appears to be part of a coordinated dependency update across packages. Let's verify the changes are complete and consistent.
✅ Verification successful
Let me generate one more verification script to check the actual usage of both hash functions to ensure the migration is complete.
Migration from hash-sum to ohash appears complete and consistent
The verification shows:
- The
ohashpackage is properly added to multiple package.json files - All hash function calls are using the imported
hashfromohash - No direct imports of
hash-sumremain in the codebase - The only remaining references to
hash-sumare in the lock file, which is expected as it maintains historical records
The migration appears to be complete and properly implemented across all packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the migration from hash-sum to ohash
# Check if there are any remaining references to hash-sum in the codebase
echo "Checking for remaining hash-sum references..."
rg -l "hash-sum"
# Check for ohash usage patterns to ensure the migration is complete
echo "Checking ohash usage patterns..."
rg -l "ohash"
# Check if hash-sum is still listed as a dependency in any package.json
echo "Checking for hash-sum in package.json files..."
fd "package.json" --exec jq -r 'select(.dependencies["hash-sum"] or .devDependencies["hash-sum"]) | input_filename'
# Verify the template.ts file mentioned in the summary has been updated
echo "Checking template.ts implementation..."
rg -A 5 "import.*hash" "packages/kit/src/template.ts"
Length of output: 1573
Script:
#!/bin/bash
# Check the actual usage of hash functions
echo "Checking hash-sum usage..."
rg -A 3 "require\(['\"](hash-sum)['\"]|from ['\"](hash-sum)['\"]"
echo "Checking ohash usage..."
rg -A 3 "hash.*=.*require\(['\"](ohash)['\"]|from ['\"](ohash)['\"]|import.*{.*hash.*}.*from ['\"](ohash)['\"]"
# Check for any direct hash-sum function calls
echo "Checking hash-sum function calls..."
rg "hash\(" --type ts --type js
# Look for any remaining references to hash-sum in the lock file
echo "Checking pnpm-lock.yaml for hash-sum..."
rg "hash-sum" pnpm-lock.yaml -A 3
Length of output: 2980
package.json (3)
20-20: LGTM! Good improvement in script definition.
The simplified command is better as it uses the locally installed knip package directly, ensuring consistent versioning and faster execution.
95-95: LGTM! Explicit knip dependency added.
The addition of knip as a direct dependency with a pinned version is good for reproducibility and matches the script command simplification.
Line range hint 1-120: Verify the impact of dependency removals.
The removal of several dependencies needs verification:
@vitejs/plugin-vue: Ensure Vue support in Vite is still maintained@eslint/js: Confirm the ESLint setup still functions correctly
✅ Verification successful
Let me generate additional scripts to verify the ESLint setup since the previous results didn't show the complete picture.
Let me check the ESLint configuration file content since we found eslint.config.mjs.
Based on the gathered information, I can now provide the final response:
Dependencies removal is safe and properly handled
- ESLint setup is maintained through
@nuxt/eslint-configand uses the new flat config format ineslint.config.mjs, so removal of@eslint/jsis safe - Vue support in Vite is properly maintained:
@vitejs/plugin-vueis still present in dependencies ofpackages/schema,packages/vite, andpackages/nuxt- It's actively used in the Vite builder implementation (
packages/vite/src/server.tsandpackages/vite/src/client.ts) - The UI templates Vite setup doesn't require Vue plugin as it's a static site build
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the impact of removed dependencies
# Check if @vitejs/plugin-vue is still required
echo "Checking for Vite configuration files..."
fd -e js -e ts -e mjs "vite.config"
# Check if @vitejs/plugin-vue is imported elsewhere
echo "Checking for @vitejs/plugin-vue usage..."
rg "@vitejs/plugin-vue"
# Check ESLint configuration
echo "Checking ESLint configuration..."
fd -e js -e json -e yaml -e yml ".eslintrc"
rg "extends.*@eslint/js"
Length of output: 3891
Script:
#!/bin/bash
# Check ESLint configuration and dependencies
# Look for ESLint config files with different naming patterns
echo "Checking for ESLint configurations..."
fd "eslint" --type f
# Check ESLint extends configurations in all possible config files
echo "Checking ESLint extends configurations..."
rg "extends.*" --type json --type yaml --type js
# Check for direct ESLint usage in scripts and configurations
echo "Checking ESLint usage in package files..."
fd "package.json" --type f --exec rg -l "eslint"
# Check if there are any ESLint plugins or configs in dependencies
echo "Checking ESLint related dependencies..."
fd "package.json" --type f --exec rg "eslint"
Length of output: 1675
Script:
#!/bin/bash
# Check ESLint configuration content and Vite setup
# Check ESLint configuration
echo "Checking ESLint configuration content..."
cat eslint.config.mjs
# Check Vite configuration in ui-templates
echo "Checking Vite configuration in ui-templates..."
cat packages/ui-templates/vite.config.ts
# Check if there are any other Vite configurations
echo "Checking for other Vite configurations..."
rg -l "defineConfig.*from.*'vite'" --type ts --type js
Length of output: 7412
packages/vite/src/vite-node.ts (1)
6-6: LGTM! Verify @nuxt/kit dependency usage.
The removal of this import aligns with the PR's objective of cleaning up unused dependencies.
Let's verify if @nuxt/kit is still needed elsewhere in the project:
✅ Verification successful
Removal of addDevServerHandler import is safe
The commented out import can be safely removed as:
addDevServerHandleris only used in the commented out function in this file- While
@nuxt/kitis widely used across the codebase, this specific function is not used anywhere else - The removal aligns with the PR's objective of cleaning up unused dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of addDevServerHandler or @nuxt/kit
rg "addDevServerHandler|from '@nuxt/kit'" --type ts
Length of output: 4869
packages/kit/src/template.ts (1)
3-3: Verify the hash function compatibility.
The change from hash-sum to ohash aligns with the dependency cleanup goals. However, we should verify that both hash functions produce compatible outputs to avoid breaking changes.
✅ Verification successful
Let me analyze the results and generate additional verification scripts to check the hash function implementation and its usage patterns more thoroughly.
Based on the comprehensive results from both script executions, I can now generate the final response:
The hash function change from hash-sum to ohash is safe to proceed.
The hash function is used in a straightforward manner for filename generation, where the output is only used to create unique identifiers. The change is safe because:
- The hash is only used as part of filename generation where exact hash value compatibility isn't critical
- The hash function is used consistently across the codebase for generating unique identifiers
- The usage pattern shows it's applied to stable inputs (template.src) where collisions would be rare
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the hash function is used elsewhere in the codebase
# and verify its usage patterns.
# Check for any other direct imports of hash-sum or ohash
echo "Checking for other hash function imports..."
rg -l "from ['\"]hash-sum['\"]|from ['\"]ohash['\"]"
# Look for hash function usage patterns to ensure compatibility
echo "Checking hash function usage patterns..."
rg "hash\(" -A 2
Length of output: 5065
Script:
#!/bin/bash
# Check the actual implementation and usage of hash function in template.ts
echo "Checking hash function implementation in template.ts..."
rg -A 5 -B 5 "hash\(" packages/kit/src/template.ts
# Check if there are any test files that might verify hash function behavior
echo "Looking for related test files..."
fd "template.test" -e ts -e js
# Check for any migration guides or documentation about this change
echo "Checking for migration or breaking changes documentation..."
fd "MIGRATION|BREAKING|CHANGELOG" -e md
Length of output: 916
| "ignoreBinaries": [ | ||
| "prepack" | ||
| ], | ||
| "ignoreWorkspaces": ["test/fixtures/basic"], | ||
| "ignoreDependencies": [ | ||
| "bing", | ||
| "uno.css" | ||
| ], |
There was a problem hiding this comment.
💡 Codebase verification
Remove "bing" from ignoreDependencies as it's not used in the project
The dependency "bing" in ignoreDependencies appears to be unnecessary as it's not actually used as a dependency in the project. While "uno.css" is legitimately used in packages/ui-templates/styles.ts, "bing" only appears in a type declaration test case and is not a real dependency.
🔗 Analysis chain
Verify the ignored dependencies exist in the project
The ignoreDependencies list includes "bing", which seems unusual. Let's verify these dependencies are actually referenced somewhere in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ignored dependencies are referenced in package.json files or source code
echo "Checking for dependencies in package.json files..."
find . -name "package.json" -exec jq -r '.dependencies + .devDependencies | keys[]' {} \; 2>/dev/null | grep -E "bing|uno.css" || echo "Not found in package.json"
echo "Checking for references in source code..."
rg -l "(?:require|import).*['\"](?:bing|uno\.css)['\"]"
Length of output: 511
Script:
#!/bin/bash
# Let's check the actual content of the files that reference these dependencies
echo "Content of test/fixtures/basic-types/types.ts:"
cat "test/fixtures/basic-types/types.ts"
echo -e "\nContent of packages/ui-templates/styles.ts:"
cat "packages/ui-templates/styles.ts"
# Also check if there are any references to these in the git history
# as they might have been removed but still in knip.json
echo -e "\nChecking recent git history for these dependencies:"
git log -n 5 --oneline --grep="bing\|uno.css"
Length of output: 30282
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
nuxt.config.ts (1)
22-22: Consider documenting the hoisting configuration.Since hoisting these specific dependencies is an optimization decision, it would be helpful to document why these packages need to be hoisted.
typescript: { shim: process.env.DOCS_TYPECHECK === 'true', - hoist: ['@vitejs/plugin-vue', 'vue-router'], + // Hoist these dependencies to optimize build performance and prevent duplicate instances + hoist: ['@vitejs/plugin-vue', 'vue-router'], },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
nuxt.config.ts(1 hunks)package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (1)
nuxt.config.ts (1)
20-23: LGTM! Verify these dependencies are still required.
The TypeScript configuration changes look correct. Since this PR aims to remove unused dependencies, let's verify these hoisted dependencies are still actively used.
✅ Verification successful
Based on the search results, I can now generate a final response:
Both hoisted dependencies need to be retained
The TypeScript configuration's hoisted dependencies are justified:
vue-routeris a core dependency used extensively throughout the codebase, particularly in the pages system and routing functionality@vitejs/plugin-vueis used in the Vite build configuration for Vue SFC compilation
Both dependencies are critical for the project's functionality and are actively used in the build toolchain, even though they appear as devDependencies rather than direct dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of hoisted dependencies
# Check for actual imports/usage of these packages in the codebase
echo "Checking @vitejs/plugin-vue usage:"
rg --type ts --type vue "@vitejs/plugin-vue"
echo "Checking direct vue-router imports:"
rg --type ts --type vue "vue-router"
# Check package.json to ensure these are direct dependencies
echo "Checking dependencies in package.json:"
cat package.json | jq '.dependencies["@vitejs/plugin-vue"], .dependencies["vue-router"]'
Length of output: 485
Script:
#!/bin/bash
# Let's try a different approach to check dependencies and their usage
# Check all dependencies in package.json
echo "Checking all dependencies sections:"
cat package.json | jq '{dependencies, devDependencies, peerDependencies}'
# Search for vue-router usage without file type restriction
echo -e "\nSearching for vue-router usage:"
rg "vue-router"
# Search for vitejs/plugin-vue usage without file type restriction
echo -e "\nSearching for @vitejs/plugin-vue usage:"
rg "@vitejs/plugin-vue"
# List all .vue files to confirm if they exist
echo -e "\nChecking for .vue files:"
fd "\.vue$"
Length of output: 35875
🔗 Linked issue
📚 Description
this tidies a list of dependencies - we need to confirm that ci still passes
Summary by CodeRabbit
New Features
rspack,ui-templates, andwebpack, each with specific entry points.Bug Fixes
hash-sumtoohash.Chores
Documentation