Update Vite version to v6 #1381
Conversation
…plugin-stylelint npm packages To support vite 6
…external @modyfi/vite-plugin-yaml npm package @modyfi/vite-plugin-yaml npm package is no longer being maintained and is blocking us from moving to vite v6
Temporarily setting `css.preprocessorOptions.scss.api: 'legacy'` for vite 6 to work. We will remove that at a later date when we update sass to use the modern js api
WalkthroughA custom Vite YAML plugin has been developed to replace an external dependency. The project has upgraded several development dependencies, including Vite and related plugins. A new local YAML transformation plugin is introduced in Changes
Sequence DiagramsequenceDiagram
participant V as Vite
participant P as YAML Plugin
participant Y as js-yaml
V->>P: Process YAML file
P->>Y: Parse YAML content
Y-->>P: Return parsed data
P-->>V: Provide transformed module
Poem
Finishing Touches
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 (
|
![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
🧹 Nitpick comments (1)
config/vite-plugin-yaml.js (1)
1-4: Consider using more specific imports.Instead of importing the entire
loadfunction, consider using a more specific import to potentially reduce bundle size.-import { load } from 'js-yaml'; +import { load as yamlLoad } from 'js-yaml';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
config/vite-plugin-yaml.js(1 hunks)package.json(3 hunks)vite.config.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: test-build
🔇 Additional comments (3)
vite.config.js (1)
33-34: Document the technical debt and create a tracking issue.The comment indicates this is a temporary solution. We should track this technical debt to ensure it's addressed in the future.
Would you like me to create a GitHub issue to track the removal of the legacy SCSS API usage? This will help ensure we don't forget about this technical debt.
package.json (2)
196-196: Verify security of new dependencies.The new dependencies
js-yamlandtosourceshould be verified for security vulnerabilities.Also applies to: 208-208
✅ Verification successful
Dependencies
js-yaml@^4.1.0andtosourceverified as secure
- No current vulnerabilities found in either package
- Both packages are well-maintained and hosted on GitHub
- js-yaml version ^4.1.0 is well above historical vulnerable versions (last known: 3.13.1)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for known vulnerabilities in new dependencies npm audit js-yaml tosource # Check for recent security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "js-yaml") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange } } }'Length of output: 795
Script:
#!/bin/bash # Check security advisories for tosource gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "tosource") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange } } }' # Check package details npm view tosource time.modified repository.urlLength of output: 442
Line range hint
177-213: Verify compatibility with Vite v6 breaking changes.The update to Vite v6 may introduce breaking changes. Please ensure all the following are verified:
- The new minimum Node.js version requirement is met
- All plugins are compatible with Vite v6
- The build output and development server work as expected
✅ Verification successful
Vite v6 upgrade is compatible with current setup
The verification shows that:
- Node.js v22.9.0 meets Vite 6's requirements
- All actively used Vite plugins are compatible with v6
- Configuration structure in
vite.config.jsfollows Vite 6 standards🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Node.js version compatibility node -v # Verify all Vite-related packages are using compatible versions npm ls vite @cypress/vite-dev-server vite-plugin-pwa vite-plugin-stylelint vite-plugin-babel vite-plugin-eslint # Check for any reported issues with these versions gh api graphql -f query=' { repository(owner: "vitejs", name: "vite") { issues(first: 5, states: OPEN, labels: ["bug"], orderBy: {field: CREATED_AT, direction: DESC}) { nodes { title url labels(first: 5) { nodes { name } } } } } }'Length of output: 614
Script:
#!/bin/bash # Search for plugin usage in configuration files echo "=== Searching for Vite plugin usage ===" rg -l "vite-plugin|@cypress/vite" --type js --type ts # Check if these plugins are mentioned in any configuration files echo -e "\n=== Checking specific configuration files ===" for file in $(fd -e js -e ts -e json -e yaml "vite|cypress" -t f); do echo "=== Content of $file ===" cat "$file" doneLength of output: 6043
config/vite-plugin-yaml.js
Outdated
| async transform(code, id) { | ||
| if (yamlExtension.test(id)) { | ||
| const yamlData = load(code, { | ||
| filename: id, | ||
| }); | ||
|
|
||
| return { | ||
| code: `const data = ${ toSource(yamlData) };\nexport default data;`, | ||
| map: { mappings: '' }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for YAML parsing.
The current implementation doesn't handle YAML parsing errors gracefully. Consider wrapping the load call in a try-catch block to provide better error messages.
async transform(code, id) {
if (yamlExtension.test(id)) {
+ try {
const yamlData = load(code, {
filename: id,
});
return {
code: `const data = ${ toSource(yamlData) };\nexport default data;`,
map: { mappings: '' },
};
+ } catch (error) {
+ const errorMessage = `Failed to parse YAML file ${id}: ${error.message}`;
+ this.error(errorMessage);
+ return null;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async transform(code, id) { | |
| if (yamlExtension.test(id)) { | |
| const yamlData = load(code, { | |
| filename: id, | |
| }); | |
| return { | |
| code: `const data = ${ toSource(yamlData) };\nexport default data;`, | |
| map: { mappings: '' }, | |
| }; | |
| } | |
| async transform(code, id) { | |
| if (yamlExtension.test(id)) { | |
| try { | |
| const yamlData = load(code, { | |
| filename: id, | |
| }); | |
| return { | |
| code: `const data = ${ toSource(yamlData) };\nexport default data;`, | |
| map: { mappings: '' }, | |
| }; | |
| } catch (error) { | |
| const errorMessage = `Failed to parse YAML file ${id}: ${error.message}`; | |
| this.error(errorMessage); | |
| return null; | |
| } | |
| } |
There was a problem hiding this comment.
Yep, this is probably a good idea 👍 we have similar try...catch error handling for our handlebars vite plugin.
There was a problem hiding this comment.
@nmajor25 Great! Thanks for confirming. Looking forward to the improved error handling.
✏️ Learnings added
Learnt from: nmajor25
PR: RoundingWell/care-ops-frontend#1381
File: config/vite-plugin-yaml.js:9-19
Timestamp: 2025-01-13T21:56:19.310Z
Learning: Vite plugins in the project should include try-catch error handling, similar to the implementation in the handlebars plugin, to provide better error messages when processing fails.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
RoundingWell Care Ops Frontend
|
||||||||||||||||||||||||||||
| Project |
RoundingWell Care Ops Frontend
|
| Branch Review |
vite-v6
|
| Run status |
|
| Run duration | 02m 51s |
| Commit |
|
| Committer | Nick Major |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
245
|
| View all changes introduced in this branch ↗︎ | |
src/scss/core/_overrides.scss
Outdated
| @@ -56,7 +56,7 @@ select { | |||
| // ------------------------------------- | |||
|
|
|||
| // remove bg image on selects from Firefox | |||
| @-moz-document url-prefix('') { | |||
| @document url-prefix('') { | |||
There was a problem hiding this comment.
Something is updating this automatically now in my ide. I think it's the new version of vite-plugin-stylelint? I'm not entirely sure.
There was a problem hiding this comment.
The mdn web doc for this rule seems to indicate that @document should be used. Which is only compatible with firefox: https://developer.mozilla.org/en-US/docs/Web/CSS/@document#browser_compatibility.
But I don't understand the context around this css. Looks like y'all added it back in the day on the initial build of the app.
There was a problem hiding this comment.
this might not be used at all anymore.. I think it was related to an old dropdown plugin, but I'm not sure.
There was a problem hiding this comment.
I actually believe we can safely remove this entire file. Had chatgpt analyze it along side our browserlist and it seems like it's all obsolete.
Pull Request Test Coverage Report for Build e6d57dca-4f1c-4253-86ef-1fb5cf1e714fDetails
💛 - Coveralls |
config/vite-plugin-yaml.js
Outdated
| @@ -0,0 +1,24 @@ | |||
| import { load } from 'js-yaml'; | |||
| import toSource from 'tosource'; | |||
There was a problem hiding this comment.
Let's remove toSource and just use JSON.stringify
src/scss/core/_overrides.scss
Outdated
| @@ -56,7 +56,7 @@ select { | |||
| // ------------------------------------- | |||
|
|
|||
| // remove bg image on selects from Firefox | |||
| @-moz-document url-prefix('') { | |||
| @document url-prefix('') { | |||
There was a problem hiding this comment.
I actually believe we can safely remove this entire file. Had chatgpt analyze it along side our browserlist and it seems like it's all obsolete.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/vite-plugin-yaml.js (1)
5-8: Consider adding plugin options for configurability.The factory function pattern is good, but consider adding configuration options for flexibility, such as:
- Custom file extensions
- YAML load options
- Output format customization
Example enhancement:
-export default function() { +export default function(options = {}) { + const { + extensions = /\.ya?ml$/, + loadOptions = {}, + } = options; return { name: 'vite-plugin-yaml', async transform(code, id) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
config/vite-plugin-yaml.js(1 hunks)package.json(3 hunks)src/scss/core/_overrides.scss(0 hunks)src/scss/outreach-core.scss(0 hunks)src/scss/provider-core.scss(0 hunks)
💤 Files with no reviewable changes (3)
- src/scss/outreach-core.scss
- src/scss/provider-core.scss
- src/scss/core/_overrides.scss
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
📓 Learnings (1)
config/vite-plugin-yaml.js (1)
Learnt from: nmajor25
PR: RoundingWell/care-ops-frontend#1381
File: config/vite-plugin-yaml.js:9-19
Timestamp: 2025-01-13T21:56:19.458Z
Learning: Vite plugins in the project should include try-catch error handling, similar to the implementation in the handlebars plugin, to provide better error messages when processing fails.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
config/vite-plugin-yaml.js (4)
1-3: LGTM! Clean import and accurate file extension matching.The implementation uses a specific import and a correct regex pattern for YAML extensions.
15-18: LGTM! Clean transformation implementation.The code correctly uses JSON.stringify and provides appropriate source map information.
19-22: 🛠️ Refactor suggestionImprove error handling to match project patterns.
Based on the project's established patterns, we should use Vite's error reporting mechanism instead of console.error.
Apply this change to align with the project's error handling patterns:
} catch (exception) { - console.error(`${ id } errored during yaml processing: `, exception); - return null; + this.error({ + id, + message: `Failed to process YAML file: ${exception.message}`, + stack: exception.stack + }); }Likely invalid or redundant comment.
5-26: Verify Vite v6 plugin API compatibility.Since this PR updates to Vite v6, we should verify that the plugin implementation follows the latest plugin API specifications.
✅ Verification successful
Plugin implementation is compatible with Vite v6 ✅
The YAML plugin follows the standard Vite plugin API structure with correct hook signatures, return values, and error handling. No changes are required for Vite v6 compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json for Vite version and related dependencies echo "Checking Vite version and dependencies..." rg '"vite":|"@vitejs/' package.json # Look for any breaking changes in plugin API echo "Checking for Vite plugin API usage..." rg 'transform.*\(.*\).*{' config/Length of output: 377
Script:
#!/bin/bash # Search for other plugin implementations echo "Checking other plugin implementations..." fd -e js -e ts --full-path ./config/ --exec grep -l "transform.*(" {} \; # Look for plugin-related documentation echo -e "\nChecking for plugin documentation..." fd -e md -e txt | xargs grep -l -i "vite.*plugin" || true # Search for specific plugin patterns echo -e "\nChecking plugin patterns..." rg "name: ['\"](vite-plugin|rollup-plugin)" --type js --type ts # Check transform hook implementations echo -e "\nChecking transform implementations..." ast-grep --pattern 'transform(code, id) { $$$ }'Length of output: 683
config/vite-plugin-yaml.js
Outdated
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Do we need this return null; if (yamlExtension.test(id)) is false? I think we are good without it. The handlebars vite plugin doesn't have it: https://github.com/RoundingWell/care-ops-frontend/blob/develop/config/vite-plugin-handlebars-loader.js
There was a problem hiding this comment.
ooh.. I think it does need it.. we should add that to the handlebars one too
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/vite-plugin-handlebars-loader.js (1)
Line range hint
1-1: Consider tracking the TODO as a separate task.Since this PR involves Vite plugin management, it might be worth creating a tracking issue for extracting this into a standalone Vite plugin package.
Would you like me to create a GitHub issue to track the extraction of this Handlebars loader into a separate Vite plugin package?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/vite-plugin-handlebars-loader.js(2 hunks)config/vite-plugin-yaml.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- config/vite-plugin-yaml.js
🔇 Additional comments (3)
config/vite-plugin-handlebars-loader.js (3)
20-20: LGTM! Explicit error handling improvement.The explicit
return nullproperly signals to Vite that this loader should be skipped when file reading fails.
30-30: LGTM! Consistent error handling.The explicit
return nullmaintains consistency with other error cases and properly handles Handlebars compilation failures.
33-34: LGTM! Explicit default case handling.The explicit default
return nullimproves clarity and maintains consistent behavior when processing non-Handlebars files.
Shortcut Story ID: [sc-57765]
Summary by CodeRabbit
Dependencies
js-yamldependency@modyfi/vite-plugin-yamlConfiguration
Styles