-
Notifications
You must be signed in to change notification settings - Fork 7.8k
chore: update dependencies and documentation, optimize build toolchain #6060
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
|
WalkthroughThe changes include updating the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
🧹 Nitpick comments (1)
package.json (1)
102-102
: Ensure engine requirement aligns with updated packageManager.The
packageManager
field was bumped to[email protected]
, but theengines.pnpm
setting still allows>=9.12.0
. This may permit older pnpm versions that lack compatibility guarantees with v10 features. For consistency and to avoid unforeseen issues, consider updating theengines.pnpm
constraint to>=10.9.0
."engines": { "node": ">=20.10.0", - "pnpm": ">=9.12.0" + "pnpm": ">=10.9.0" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (1)
package.json (1)
102-102
: Verify lockfile compatibility with [email protected].Changing the packageManager may require regenerating
pnpm-lock.yaml
to ensure the lockfile schema and dependency resolutions align with pnpm v10.9.0. Please confirm that no lockfile issues arise on CI and consider running a clean install:pnpm install --force
…ibility and stability - Update Node version to 22.1.0 - Updated pnpm version to 10.10.0 - Fixed syntax error in prettier command in lintstagedrc - Update dependent versions in pnpm-lock.yaml to ensure consistency - Update format and content in README documents to improve readability
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
🧹 Nitpick comments (1)
scripts/clean.mjs (1)
35-56
: Consider adding a deletion summaryWhile the current logging improvements are good, consider adding a summary at the end that counts how many items were deleted. This would provide users with a quick overview of the cleanup operation.
try { await cleanTargetsRecursively(rootDir, cleanupTargets); + console.log('Cleanup process completed successfully.'); - console.log('Cleanup process completed successfully.'); + console.log(`Cleanup process completed successfully. Run with --verbose for details.`); } catch (error) { console.error(`Unexpected error during cleanup: ${error.message}`); process.exit(1); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.lintstagedrc.mjs
(1 hunks).node-version
(1 hunks).vscode/settings.json
(3 hunks)README.ja-JP.md
(5 hunks)README.md
(6 hunks)README.zh-CN.md
(4 hunks)internal/vite-config/src/typing.ts
(1 hunks)package.json
(1 hunks)pnpm-workspace.yaml
(5 hunks)scripts/clean.mjs
(2 hunks)scripts/deploy/Dockerfile
(2 hunks)scripts/turbo-run/README.md
(1 hunks)scripts/turbo-run/src/run.ts
(1 hunks)scripts/vsh/README.md
(1 hunks)scripts/vsh/src/check-circular/index.ts
(1 hunks)scripts/vsh/src/check-dep/index.ts
(1 hunks)scripts/vsh/src/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- package.json
- .node-version
- .lintstagedrc.mjs
- scripts/deploy/Dockerfile
- .vscode/settings.json
- scripts/vsh/README.md
- pnpm-workspace.yaml
- scripts/turbo-run/README.md
- README.zh-CN.md
- README.md
- README.ja-JP.md
🧰 Additional context used
🧬 Code Graph Analysis (2)
scripts/vsh/src/check-dep/index.ts (1)
internal/node-utils/src/monorepo.ts (1)
getPackages
(46-46)
internal/vite-config/src/typing.ts (1)
internal/vite-config/src/plugins/inject-metadata.ts (1)
config
(79-105)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (35)
scripts/turbo-run/src/run.ts (1)
28-28
: Type parameter refinement looks good.The change from
select<any, string>
toselect<string>
appropriately narrows the type parameter to just string, which matches the expected return type. This change improves type safety without altering the function's behavior and aligns with modern TypeScript best practices.scripts/clean.mjs (8)
2-2
: Good addition of path normalizationUsing
normalize
fromnode:path
is a great improvement as it ensures path consistency across different operating systems, enhancing the script's reliability.
9-9
: Documentation properly updatedThe JSDoc comment has been correctly updated to reflect that
targets
is now an array of strings, which aligns with how the function is actually used.
16-18
: Improved path handling and code organizationMoving the
lstat
call earlier and applying path normalization are excellent improvements. This reorganization allows for better control flow and ensures consistent path handling across different platforms.
23-24
: Clearer conditional logicThe refactored conditional structure makes the code more readable by explicitly checking if the item is a directory before recursion. The added comment also improves code clarity.
27-31
: Enhanced error handlingThe improved error message now provides contextual information about where the error occurred, making debugging much easier. This is a significant improvement in error reporting.
40-40
: Good use of array spread for immutabilityUsing the spread operator to create a copy of the targets array before potentially modifying it is a good practice that maintains immutability of the original array.
51-51
: Improved user feedbackAdding a success message upon completion provides better feedback to users, making the script more user-friendly.
54-54
: Proper error handling with process exitAdding
process.exit(1)
ensures that the script correctly signals failure to any calling processes, which is essential for proper error handling in automated workflows.internal/vite-config/src/typing.ts (8)
6-22
: Well-structured JSDoc documentation with effective examplesThe JSDoc comments for the
IImportMap
interface provide clear explanations and include practical examples that demonstrate the implementation. This documentation style effectively communicates the purpose and usage of the interface.Note that the JSDoc comments are in Chinese while the property comments are in English. Consider maintaining a consistent language throughout all documentation for better maintainability.
32-49
: Documentation includes clear examples for PrintPluginOptionsThe JSDoc documentation for
PrintPluginOptions
is well-structured with good examples showing the expected format for theinfoMap
property. This documentation style helps developers understand how to use this configuration option.
51-73
: Default values clearly documented in JSDocThe JSDoc comments for
NitroMockPluginOptions
,ArchiverPluginOptions
, andImportmapPluginOptions
include default values, which is excellent practice. This helps developers understand what values will be used if they don't explicitly set these options.Also applies to: 75-90, 92-120
139-175
: CommonPluginOptions provides a well-structured base interfaceThe
CommonPluginOptions
interface serves as a solid foundation for other plugin configurations. The properties are well-documented with clear descriptions and default values. This approach promotes consistency across different plugin configurations.
177-277
: Comprehensive ApplicationPluginOptions with detailed property documentationThe
ApplicationPluginOptions
interface extendsCommonPluginOptions
and provides a comprehensive set of application-specific configuration options. Each property has detailed documentation explaining its purpose, default value, and in some cases, additional description of functionality. This level of detail is excellent for developer experience.
279-290
: LibraryPluginOptions remains focused on library-specific needsThe
LibraryPluginOptions
interface is appropriately focused on library-specific configuration needs, currently only adding thedts
option for TypeScript declaration file generation. This maintains a clean separation between application and library configurations.
302-328
: Well-defined asynchronous configuration functionsThe type definitions for
DefineApplicationOptions
,DefineLibraryOptions
, andDefineConfig
effectively establish a structured pattern for asynchronous configuration functions. This approach allows for dynamic configuration generation based on the build environment.
1-343
: Comprehensive type system for Vite plugin configurationThis file adds a well-structured, comprehensive type system for Vite plugin configurations. The interfaces are logically organized with a common base interface and specialized extensions for different use cases. The detailed JSDoc comments make the code self-documenting.
While this file seems to be a substantial addition of new type definitions rather than a direct change related to the package manager update mentioned in the PR description, it significantly improves type safety and developer experience for working with Vite plugins in the project.
scripts/vsh/src/index.ts (4)
5-5
: Good practice: Properly sourcing version from package.jsonThis change ensures the CLI version is consistently sourced from package.json rather than being hardcoded.
12-19
: Well-structured command descriptionsCentralizing command descriptions in a constant makes them easier to maintain and ensures consistency across the CLI. The use of
as const
provides proper type safety.
21-64
: Good refactoring: Improved CLI initialization with error handlingRefactoring the CLI initialization into an async
main
function with proper error handling significantly improves code quality. The structure is clean with:
- Try-catch blocks for granular error handling
- Centralized command registration
- Clear command feedback for invalid inputs
- Well-organized CLI setup with usage and version information
66-74
: Proper async error handlingThe top-level invocation with promise error handling ensures all unhandled promise rejections are properly caught and reported, which is an important improvement for CLI reliability.
scripts/vsh/src/check-circular/index.ts (7)
9-24
: Good refactoring: Centralized configurationMoving configuration options to a
DEFAULT_CONFIG
constant improves maintainability by centralizing all default settings in one place. The use ofas const
ensures type safety for the configuration object.
29-33
: Well-designed configuration interfaceThe
CheckCircularConfig
interface properly documents the expected structure with optional properties, making the configuration options clear to users.
41-42
: Performance improvement with cachingAdding a caching mechanism is a good optimization that helps avoid redundant circular dependency checks, especially when running multiple commands in sequence.
48-59
: Improved output formattingThe
formatCircles
function provides clear, structured output that makes circular dependencies easier to understand and debug.
69-140
: Enhanced circular dependency checking with configuration optionsThe refactored
checkCircular
function is well-structured with:
- Configuration merging with defaults
- Dynamic ignore pattern generation
- Efficient caching mechanism
- Proper filtering of files based on extensions
- Improved result formatting and handling
146-167
: Extended CLI options for better user controlThe command definition now includes additional options for fine-tuning circular dependency checks, providing better flexibility for users while maintaining reasonable defaults.
170-170
: Appropriate type exportsExporting the
CheckCircularConfig
type allows consumers of this module to access the type definitions, which is good practice for TypeScript modules.scripts/vsh/src/check-dep/index.ts (7)
7-36
: Good refactoring: Comprehensive default configurationThe
DEFAULT_CONFIG
object centralizes all configuration settings with clear categorization:
- Dependencies to ignore by pattern
- Packages to completely skip
- File patterns to ignore
This approach improves maintainability and makes the configuration more self-documenting.
38-55
: Well-designed type definitionsThe interfaces clearly define the structure of dependency check results, configuration options, and package information, providing good type safety and documentation.
61-74
: Improved result cleaning with clear logicThe
cleanDepcheckResult
function properly handles special cases:
- Removing local file-prefixed dependencies
- Filtering out path-based missing dependencies
- Cleaning up empty result entries
This results in more accurate and relevant dependency check reports.
81-110
: Enhanced result formatting for better readabilityThe
formatDepcheckResult
function provides clear, structured output that categorizes issues into:
- Missing dependencies with their file references
- Unused regular dependencies
- Unused dev dependencies
The added emojis and indentation improve readability.
116-162
: Robust dependency checking with error handlingThe refactored
runDepcheck
function includes several improvements:
- Configuration merging with defaults
- Proper tracking of issues across packages
- Clear success messaging when no issues are found
- Comprehensive error handling
168-193
: Extended CLI options for better user controlThe command definition now includes additional options for customizing dependency checks:
- Specifying packages to ignore
- Defining dependency patterns to ignore
- Setting file patterns to exclude
This provides better flexibility while maintaining sensible defaults.
195-195
: Appropriate type exportsExporting the
DepcheckConfig
type alongside the command definition function allows consumers to access the type definitions, following good TypeScript practices.
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)
packages/effects/access/src/accessible.ts (1)
42-43
: Consider an alternative to the delete operatorThe static analysis tool flagged the use of the
delete
operator which can impact performance. While this isn't part of your current changes, consider refactoring in a future update.- delete route.component; + route.component = undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/effects/access/src/accessible.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/effects/access/src/accessible.ts
[error] 47-47: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
🔇 Additional comments (1)
packages/effects/access/src/accessible.ts (1)
45-55
: Logical improvement to prevent stale route data - LGTM!This change appropriately reverses the logic to update existing routes before adding new ones, which prevents stale route data during user switches. Without this fix, switching users could lead to outdated routes persisting in the navigation, potentially causing 404 errors (as mentioned in the comment on line 47 about
homePath
).🧰 Tools
🪛 Biome (1.9.4)
[error] 47-47: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
#6060) * chore: update packageManager version to [email protected] for compatibility improvements * chore: Update dependent versions and configurations to improve compatibility and stability - Update Node version to 22.1.0 - Updated pnpm version to 10.10.0 - Fixed syntax error in prettier command in lintstagedrc - Update dependent versions in pnpm-lock.yaml to ensure consistency - Update format and content in README documents to improve readability * fix: lint error
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