Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR migrates the repository's formatter from Prettier to Oxfmt: it removes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 62-63: Remove the unused "prettier" devDependency from
package.json (the entry named "prettier" in devDependencies) because formatting
is handled by "oxfmt" (see the "format" script) and only
"eslint-config-prettier" is needed for ESLint; update package.json to delete the
"prettier" entry so installs and toolchain ambiguity are reduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f7f3966-b47b-4c86-98d7-da49fbd2e777
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/.kodiak.toml.oxfmtrc.json.prettierignore.prettierrc.jsonpackage.json
💤 Files with no reviewable changes (2)
- .prettierrc.json
- .prettierignore
|
awesome, yes please! LGTM @TheAlexLichter but it's failing in CI. any ideas? 🤔 |
|
npm/cli#4828 😮💨 @serhalp Addded a workaround which needs to be tested in the CI workflow. EDIT: Seems like it worked. formatting down from 11s to 2s 👏🏻 |
25f4b62 to
81954cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Around line 123-125: Update the CircleCI job that installs the native binding
by pinning the binding package version to match oxfmt: replace the bare package
install of `@oxfmt/binding-linux-x64-gnu` used in the run step with a versioned
install (e.g., `@oxfmt/binding-linux-x64-gnu`@^0.43.0) so the native binding stays
aligned with oxfmt; locate the run step that currently contains "npm i -D
`@oxfmt/binding-linux-x64-gnu`" and change it to include the version specifier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0292d7de-98f9-4874-8f59-8ab8dfac57d7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.circleci/config.ymleslint.config.jspackage.json
💤 Files with no reviewable changes (1)
- eslint.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
| # This is necessary to work around platform-specific optional dependencies bug | ||
| # (https://github.com/npm/cli/issues/4828). | ||
| - run: npm i -D @oxfmt/binding-linux-x64-gnu |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that oxfmt and its binding package exist on npm and check version alignment
echo "=== Checking oxfmt package ==="
curl -s https://registry.npmjs.org/oxfmt | jq '{name: .name, latest: .["dist-tags"].latest, versions: (.versions | keys | .[-5:])}'
echo ""
echo "=== Checking `@oxfmt/binding-linux-x64-gnu` package ==="
curl -s https://registry.npmjs.org/@oxfmt/binding-linux-x64-gnu | jq '{name: .name, latest: .["dist-tags"].latest, versions: (.versions | keys | .[-5:])}'Repository: netlify/build
Length of output: 428
Pin the binding version to match oxfmt.
While @oxfmt/binding-linux-x64-gnu and oxfmt are currently both at version 0.43.0, pinning the binding version to @^0.43.0 ensures they stay in sync. Native bindings should explicitly declare version alignment with their parent packages to prevent future mismatches if versions diverge.
Proposed fix
# This is necessary to work around platform-specific optional dependencies bug
# (https://github.com/npm/cli/issues/4828).
- - run: npm i -D `@oxfmt/binding-linux-x64-gnu`
+ - run: npm i -D `@oxfmt/binding-linux-x64-gnu`@^0.43.0📝 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.
| # This is necessary to work around platform-specific optional dependencies bug | |
| # (https://github.com/npm/cli/issues/4828). | |
| - run: npm i -D @oxfmt/binding-linux-x64-gnu | |
| # This is necessary to work around platform-specific optional dependencies bug | |
| # (https://github.com/npm/cli/issues/4828). | |
| - run: npm i -D `@oxfmt/binding-linux-x64-gnu`@^0.43.0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.circleci/config.yml around lines 123 - 125, Update the CircleCI job that
installs the native binding by pinning the binding package version to match
oxfmt: replace the bare package install of `@oxfmt/binding-linux-x64-gnu` used in
the run step with a versioned install (e.g.,
`@oxfmt/binding-linux-x64-gnu`@^0.43.0) so the native binding stays aligned with
oxfmt; locate the run step that currently contains "npm i -D
`@oxfmt/binding-linux-x64-gnu`" and change it to include the version specifier.
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #7015
Faster formatting goes brrrrr
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)