fix(ui~cli): windows path normalization for setup plugins #1541
Conversation
…dPath in plugin setup commands
SutuSebastian
added
🐛 bug
🦋 Changeset detectedLatest commit: 44be4e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request replaces all instances of Node.js's Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Plugin as setupPlugin Function
participant Norm as joinNormalizedPath
participant Config as Plugin Config Handler
CLI->>Plugin: Invoke plugin setup
Plugin->>Norm: Normalize path (pluginPath, identifier)
Norm-->>Plugin: Return normalized path
Plugin->>Config: Pass normalized path for config update
Config-->>Plugin: Configuration updated
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/ui/src/cli/commands/plugins/setup-plugin-astro.tsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/ui/src/cli/commands/plugins/setup-plugin-farm.tsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/ui/src/cli/commands/plugins/setup-plugin-bun.tsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ 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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
SutuSebastian
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/shiny-parents-raise.md (1)
5-5: Fix typo in changeset message.There's a minor typo in the changeset description: "normalzation" should be "normalization".
-fix(ui~cli): path normalzation for windows support (setup plugins paths) +fix(ui~cli): path normalization for windows support (setup plugins paths)packages/ui/src/cli/commands/setup-tailwind.ts (1)
61-63: Consider using joinNormalizedPath for sourceDirectivePath.There's still a manual path normalization happening for the sourceDirectivePath that could potentially benefit from using the joinNormalizedPath utility for consistency.
-const sourceDirectivePath = path - .join(path.relative(path.dirname(file), process.cwd()), classListFilePath) - .replace(/\\/g, "/"); +const sourceDirectivePath = joinNormalizedPath( + path.relative(path.dirname(file), process.cwd()), + classListFilePath +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.changeset/shiny-parents-raise.md(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-astro.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-bun.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-farm.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-modernjs.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-nextjs.ts(2 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-parcel.ts(2 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-rolldown.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-rollup.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-rsbuild.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-rspack.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-tanstack-start.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-vite.ts(1 hunks)packages/ui/src/cli/commands/plugins/setup-plugin-webpack.ts(1 hunks)packages/ui/src/cli/commands/setup-tailwind.ts(3 hunks)packages/ui/src/cli/utils/__tests__/normalize-path.test.ts(1 hunks)packages/ui/src/cli/utils/normalize-path.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (15)
packages/ui/src/cli/commands/plugins/setup-plugin-parcel.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-tanstack-start.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-webpack.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-nextjs.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-bun.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-rsbuild.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-vite.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-modernjs.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-rspack.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-farm.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-astro.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-rolldown.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/setup-tailwind.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/commands/plugins/setup-plugin-rollup.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (1)
joinNormalizedPath(21-23)
packages/ui/src/cli/utils/__tests__/normalize-path.test.ts (1)
packages/ui/src/cli/utils/normalize-path.ts (2)
normalizeImportPath(10-12)joinNormalizedPath(21-23)
🔇 Additional comments (17)
packages/ui/src/cli/commands/plugins/setup-plugin-tanstack-start.ts (1)
3-3: Path normalization looks good.The replacement of
path.joinwithjoinNormalizedPathfor plugin import paths will help ensure consistent path formatting across platforms, particularly for Windows where backslashes can cause issues in import statements.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-parcel.ts (2)
4-4: Path normalization for import path looks good.Correctly replaces
path.joinwithjoinNormalizedPathfor the plugin import path to ensure consistent path formatting across platforms.Also applies to: 42-42
11-11:❓ Verification inconclusive
Consider using joinNormalizedPath for other paths if needed.
I notice that other
path.joininstances in this file weren't replaced. If these paths are used for imports or in places where path format matters for cross-platform compatibility, consider usingjoinNormalizedPathfor them as well.Also applies to: 23-23, 44-44
🏁 Script executed:
#!/bin/bash # Check if other path.join instances in this file are used in similar contexts # where normalization would be beneficial # Find how the variables parcelConfigDir, parcelConfigFilePath, and parcelReporterFilePath are used rg -A 5 "parcelConfigDir|parcelConfigFilePath|parcelReporterFilePath" --type ts packages/ui/src/cli/commands/plugins/setup-plugin-parcel.tsLength of output: 2637
Path Normalization Considerations in setup-plugin-parcel.ts
After verifying the usage of
path.joinin this file, it appears that:
parcelConfigDir,parcelConfigFilePath, andparcelReporterFilePath: These are used exclusively for file system operations (e.g., creating directories, reading and writing files). For these purposes, the native behavior ofpath.join(which handles OS-specific separators) is appropriate.pluginImportPath: Already usesjoinNormalizedPathbecause it’s used in a module import context where a normalized, POSIX-style path is beneficial.Recommendation:
- If the file system paths (lines 11, 23, and 44) are solely for interacting with the file system, no changes are necessary.
- However, if any of these paths might later be used in contexts where cross-platform path normalization (like in module imports) is required, then consider refactoring them to use
joinNormalizedPath.packages/ui/src/cli/commands/plugins/setup-plugin-rolldown.ts (1)
3-3: Path normalization looks good.The replacement of
path.joinwithjoinNormalizedPathfor the plugin import path will ensure consistent path formatting across all platforms, addressing Windows compatibility issues.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-rspack.ts (1)
3-3: Good implementation of cross-platform path normalization.The change from the native
path.jointo the customjoinNormalizedPathfunction is appropriate for ensuring consistent path handling across different operating systems, particularly Windows where backslashes could cause issues in import paths.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-webpack.ts (1)
3-3: Good implementation of cross-platform path normalization.The replacement of the native
path.joinwithjoinNormalizedPathensures proper path normalization across different operating systems, especially on Windows where backslashes in paths could cause import issues.Also applies to: 9-9
packages/ui/src/cli/utils/normalize-path.ts (1)
1-23: Well-designed utility functions with good documentation.The implementation of these path normalization utilities is clean and effective:
normalizeImportPathcorrectly handles the conversion of backslashes to forward slashesjoinNormalizedPathcombines path.join with normalization in a logical way- JSDoc comments clearly explain the purpose and usage of both functions
This is a solid approach to ensuring consistent path handling across different operating systems, particularly addressing Windows compatibility issues where backslashes in paths can cause problems in import statements.
packages/ui/src/cli/commands/plugins/setup-plugin-farm.ts (1)
3-3: Good implementation of cross-platform path normalization.The change from
path.jointojoinNormalizedPathensures consistent handling of paths across different operating systems, particularly improving Windows compatibility by normalizing backslashes to forward slashes for import paths.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-nextjs.ts (1)
4-4: Good improvement for Windows path compatibility.The introduction of
joinNormalizedPathis a good approach to ensure path normalization consistency across different operating systems, especially Windows. This will help prevent issues related to backslash vs forward slash path separators.Also applies to: 16-16
packages/ui/src/cli/commands/plugins/setup-plugin-modernjs.ts (1)
5-5: Consistent usage of normalized path function.The implementation correctly replaces path.join with joinNormalizedPath, maintaining consistency with other plugin setup files and ensuring proper path normalization for cross-platform compatibility.
Also applies to: 11-11
packages/ui/src/cli/commands/plugins/setup-plugin-astro.ts (1)
3-3: Path normalization correctly implemented.The change appropriately implements the joinNormalizedPath function to handle cross-platform path normalization, which should resolve Windows-specific path issues.
Also applies to: 9-9
packages/ui/src/cli/commands/setup-tailwind.ts (1)
7-7: Path normalization correctly applied to plugin paths.The implementation correctly uses joinNormalizedPath to normalize the plugin-related paths, which is consistent with the approach in other files.
Also applies to: 60-60, 122-122
packages/ui/src/cli/commands/plugins/setup-plugin-rollup.ts (1)
3-3: Great improvement for cross-platform compatibility!The switch from
path.jointojoinNormalizedPathwill help ensure consistent path formatting across different operating systems, especially Windows where backslashes in paths can cause issues. This change will normalize the plugin import path to use forward slashes which is more compatible with JavaScript import statements.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-vite.ts (1)
3-3: Good path normalization implementationConsistent with the other plugin setup files, this properly replaces
path.joinwithjoinNormalizedPathto ensure Windows compatibility.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-rsbuild.ts (1)
3-3: Clean implementation of path normalizationThe replacement of
path.joinwithjoinNormalizedPathmaintains functionality while ensuring consistent path formatting across operating systems.Also applies to: 9-9
packages/ui/src/cli/commands/plugins/setup-plugin-bun.ts (1)
4-4: Properly normalized paths for Bun setupGood implementation of the path normalization strategy. This is especially important for the Bun setup where the plugin path is used in multiple places - both for checking if it's already in the plugins array and for setting the pluginImportPath in the updateBuildConfig call.
Also applies to: 12-12
packages/ui/src/cli/utils/__tests__/normalize-path.test.ts (1)
1-44: Well-structured and comprehensive test coverage!The test suite provides excellent coverage for the path normalization utilities, ensuring proper behavior across different operating systems. The organization is clear and logical, with distinct test cases for various scenarios.
A few suggestions to make the tests even more robust:
- Consider adding tests for paths with special characters (spaces, Unicode)
- Add tests for paths with ".." and "." segments to ensure proper normalization
- Test with absolute path edge cases (e.g., root paths like "/" or "C:\")
These utilities will be very helpful for ensuring consistent path handling across different platforms, particularly for Windows compatibility.
Summary by CodeRabbit