chore: quality-tightening (oxfmt + drop prettier/eslint + tsgo + husky self-heal) - #95
Conversation
…y self-heal) This repo was already partially migrated (had oxlint, husky 9, lint-staged, vitest). Finishes the alignment with the rest of the rollout: - prettier (root) + prettier (functions/) -> oxfmt across the workspace - functions/ workspace eslint + @typescript-eslint/* + jest -> dropped entirely; the workspace now uses the root oxlint via 'yarn lint'. - new: oxlint plugins extended to typescript + vitest + unicorn + oxc (was just baseline correctness rules) - new: tsgo --noEmit as default 'typecheck'; tsc kept as 'typecheck:tsc' fallback - husky 9: existing /sh/ hook + 'husky install' replaced with the standard scripts/ensure-husky.mjs self-heal pattern + lint-staged + gitleaks. Drops the broken 'exec >/dev/tty' redirect that fails in non-interactive shells. - npm 'shellcheck' package dropped (playbook trap #2: it's broken and corrupts actionlint output). mise.toml adds shellcheck plus actionlint and gitleaks as managed binaries. - CI: existing main.yml steps re-ordered (Format/Lint/Typecheck/ Test) and 'yarn format' (which writes) replaced with 'yarn format:check' (which doesn't). codecov-action v4.0.1 -> v5. oxlint config tunes: - typescript/no-extraneous-class off (the codebase uses static-only classes as namespaces, idiomatic in this repo) - no-await-in-loop off (Firestore migrations and rate-limited HTTP loops are single-threaded by design) 13 pre-existing typescript/no-explicit-any warnings remain (mostly in firebase-functions handler signatures and json deserialisation). Per playbook trap #22 the lint script drops --deny-warnings. Verified locally: lint 0/13, format clean, typecheck clean, test 7/7.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR consolidates a tooling migration from Prettier to oxfmt/oxlint and reorganizes the development workflow from Turbo-based to Husky-based git hooks, with updated GitHub Actions, package scripts, and new helper utilities to manage Husky initialization across environments. ChangesTooling & Workflow Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![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 (5)
.oxfmtrc.json (1)
1-16: 💤 Low valueAdd a
$schemafield for editor validation and autocomplete.Adding a
$schemafield enables editor validation and autocomplete. The.oxlintrc.jsonin this PR already sets one; the same treatment here would keep the two configs consistent.✨ Suggested addition
{ + "$schema": "./node_modules/oxfmt/configuration_schema.json", "semi": true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.oxfmtrc.json around lines 1 - 16, Add a top-level "$schema" field to the .oxfmtrc.json to enable editor validation/autocomplete; edit the JSON object that currently contains keys like "semi", "singleQuote", "trailingComma", etc., and insert a "$schema" property (matching the same schema used in .oxlintrc.json) as the first or near-first property so editors can pick it up and validate/autocomplete the config.mise.toml (1)
4-6: ⚡ Quick winConsider pinning tool versions for reproducibility.
actionlint,shellcheck, andgitleaksare set to"latest", unlikenodeandyarnwhich are version-pinned. A new release of any of these tools could introduce stricter rules or breaking changes that silently break developer environments or CI.🔧 Suggested fix
-actionlint = "latest" -shellcheck = "latest" -gitleaks = "latest" +actionlint = "1.7.7" +shellcheck = "0.10.0" +gitleaks = "8.27.2"Replace the version strings above with the current versions you have validated locally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mise.toml` around lines 4 - 6, Replace the floating "latest" pins for the linters with concrete, tested versions: update the actionlint, shellcheck, and gitleaks entries so their values are the specific version strings you validated locally (e.g., "vX.Y.Z" or the exact release number), ensuring they match the format used for node/yarn; change the keys actionlint, shellcheck, and gitleaks in mise.toml to those fixed versions.package.json (2)
28-29: 💤 Low value
actionlintin lint-staged requires out-of-band installation.
actionlintis not listed indevDependenciesand is managed bymise.toml. Developers withoutmise(or with a stalemisecache) will get acommand not foundfailure when staging.github/workflows/files. Consider adding a comment or README note about this prerequisite, or wrapping the call with a guard that skips gracefully whenactionlintisn't in PATH.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 28 - 29, The lint-staged entry ".github/workflows/*.@(yml|yaml)": "actionlint" calls actionlint but actionlint isn't listed in devDependencies (it's managed out-of-band by mise), causing failures for developers without mise; fix by either adding actionlint to devDependencies in package.json so it is installed for all contributors, or change the lint-staged command for the ".github/workflows/*.@(yml|yaml)" glob to a guarded invocation that skips gracefully when actionlint isn't on PATH (e.g., a shell wrapper that checks for the command before running), and optionally add a short README note about the mise prerequisite if you keep the external management.
54-54: 💤 Low valuePin
@typescript/native-previewto an exact version for predictable CI/local parity.
@typescript/native-previewis a nightly preview that may lack functionality present in the stable compiler, and unexpected breakage is possible when the lockfile is regenerated. Thefunctionsworkspace typecheck script invokestsgofrom this package. The current^range means a freshyarn install(after regenerating the lock file) will pull the latest nightly automatically. Pinning to an exact version (removing the caret) would prevent unintended version differences between CI and local builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 54, The dependency declaration for "@typescript/native-preview" uses a caret range which allows updating to newer nightly builds and can cause CI/local parity issues; update the package.json entry for "@typescript/native-preview" to an exact version (remove the leading "^") so installs always use the same nightly, and ensure any references to "tsgo" in the functions workspace typecheck script remain compatible with that pinned version..github/workflows/main.yml (1)
54-64: ⚖️ Poor tradeoffConsider splitting quality gates into separate jobs for better CI feedback.
All four checks (Format → Lint → Typecheck → Test) run sequentially in the same job. A failure in Format blocks visibility of Lint/Typecheck/Test results until Format is fixed. Splitting these into parallel jobs (or at minimum using
continue-on-error: trueon the non-test steps) would surface all failures at once and speed up the feedback loop.This is especially relevant now that Format and Lint are independent of Test infrastructure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/main.yml around lines 54 - 64, Split the single job that runs the "Format", "Lint", "Typecheck", and "Test" steps into separate GitHub Actions jobs so failures are reported independently and can run in parallel: create standalone jobs named e.g. "format-check", "lint", "typecheck", and "test" that each run the corresponding command (yarn format:check, yarn lint, yarn typecheck, yarn coverage), or if you prefer keep them in one job add continue-on-error: true to the non-test steps ("Format", "Lint", "Typecheck") so they don't block subsequent checks; ensure job names match the existing step labels ("Format", "Lint", "Typecheck", "Test") so it's easy to locate and verify the changes in main.yml.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.yml:
- Around line 17-18: The step that extracts node version using the run command
(the `run: echo "version=$(grep -E '^node\s*=' mise.toml | sed -E
's/.*"([^"]+)".*/\1/')" >> "$GITHUB_OUTPUT"` line) only matches double-quoted
values; change the sed/regex to accept optional surrounding quotes (double or
single) or bare values so it reliably captures node = "20.x", node = '20.x', or
node = 20, and replace the existing pattern in that run step; then apply the
identical fix to the repeated extraction steps in the build, testDeploy, and
deploy jobs so all actions/setup-node invocations receive a robust version
value.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Around line 54-64: Split the single job that runs the "Format", "Lint",
"Typecheck", and "Test" steps into separate GitHub Actions jobs so failures are
reported independently and can run in parallel: create standalone jobs named
e.g. "format-check", "lint", "typecheck", and "test" that each run the
corresponding command (yarn format:check, yarn lint, yarn typecheck, yarn
coverage), or if you prefer keep them in one job add continue-on-error: true to
the non-test steps ("Format", "Lint", "Typecheck") so they don't block
subsequent checks; ensure job names match the existing step labels ("Format",
"Lint", "Typecheck", "Test") so it's easy to locate and verify the changes in
main.yml.
In @.oxfmtrc.json:
- Around line 1-16: Add a top-level "$schema" field to the .oxfmtrc.json to
enable editor validation/autocomplete; edit the JSON object that currently
contains keys like "semi", "singleQuote", "trailingComma", etc., and insert a
"$schema" property (matching the same schema used in .oxlintrc.json) as the
first or near-first property so editors can pick it up and validate/autocomplete
the config.
In `@mise.toml`:
- Around line 4-6: Replace the floating "latest" pins for the linters with
concrete, tested versions: update the actionlint, shellcheck, and gitleaks
entries so their values are the specific version strings you validated locally
(e.g., "vX.Y.Z" or the exact release number), ensuring they match the format
used for node/yarn; change the keys actionlint, shellcheck, and gitleaks in
mise.toml to those fixed versions.
In `@package.json`:
- Around line 28-29: The lint-staged entry ".github/workflows/*.@(yml|yaml)":
"actionlint" calls actionlint but actionlint isn't listed in devDependencies
(it's managed out-of-band by mise), causing failures for developers without
mise; fix by either adding actionlint to devDependencies in package.json so it
is installed for all contributors, or change the lint-staged command for the
".github/workflows/*.@(yml|yaml)" glob to a guarded invocation that skips
gracefully when actionlint isn't on PATH (e.g., a shell wrapper that checks for
the command before running), and optionally add a short README note about the
mise prerequisite if you keep the external management.
- Line 54: The dependency declaration for "@typescript/native-preview" uses a
caret range which allows updating to newer nightly builds and can cause CI/local
parity issues; update the package.json entry for "@typescript/native-preview" to
an exact version (remove the leading "^") so installs always use the same
nightly, and ensure any references to "tsgo" in the functions workspace
typecheck script remain compatible with that pinned version.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18e9531a-ed2e-4956-a7fa-7bddae7e9f36
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.github/workflows/main.yml.husky/pre-commit.oxfmtrc.json.oxlintrc.json.prettierignore.prettierrc.json.yarnrc.ymlDEVELOPMENT.mdfunctions/package.jsonfunctions/src/config/credential.tsmise.tomlpackage.jsonpublic/404.htmlpublic/index.htmlscripts/ensure-husky.mjs
💤 Files with no reviewable changes (3)
- .prettierrc.json
- DEVELOPMENT.md
- .prettierignore
| run: echo "version=$(grep -E '^node\s*=' mise.toml | sed -E 's/.*"([^"]+)".*/\1/')" >> | ||
| "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Node-version extraction relies on double-quoted value format in mise.toml.
The sed pattern s/.*"([^"]+)".*/\1/ requires the value to be double-quoted (e.g. node = "20.x"). If mise.toml is ever updated to use single quotes or bare integers, sed returns an empty string and actions/setup-node fails silently with a confusing error. The same pattern is repeated in the build, testDeploy, and deploy jobs (Lines 83-84, 133-134, 196-197).
🛡️ More robust extraction
- run: echo "version=$(grep -E '^node\s*=' mise.toml | sed -E 's/.*"([^"]+)".*/\1/')" >>
- "$GITHUB_OUTPUT"
+ run: |
+ version=$(grep -E '^node\s*=' mise.toml | sed -E 's/.*["'"'"']([^"'"'"']+)["'"'"'].*/\1/')
+ echo "version=$version" >> "$GITHUB_OUTPUT"
+ if [ -z "$version" ]; then echo "::error::Could not parse node version from mise.toml"; exit 1; fiApply the same fix to the identical steps in build (Line 83), testDeploy (Line 133), and deploy (Line 196).
📝 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.
| run: echo "version=$(grep -E '^node\s*=' mise.toml | sed -E 's/.*"([^"]+)".*/\1/')" >> | |
| "$GITHUB_OUTPUT" | |
| run: | | |
| version=$(grep -E '^node\s*=' mise.toml | sed -E 's/.*["'"'"']([^"'"'"']+)["'"'"'].*/\1/') | |
| echo "version=$version" >> "$GITHUB_OUTPUT" | |
| if [ -z "$version" ]; then echo "::error::Could not parse node version from mise.toml"; exit 1; fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/main.yml around lines 17 - 18, The step that extracts node
version using the run command (the `run: echo "version=$(grep -E '^node\s*='
mise.toml | sed -E 's/.*"([^"]+)".*/\1/')" >> "$GITHUB_OUTPUT"` line) only
matches double-quoted values; change the sed/regex to accept optional
surrounding quotes (double or single) or bare values so it reliably captures
node = "20.x", node = '20.x', or node = 20, and replace the existing pattern in
that run step; then apply the identical fix to the repeated extraction steps in
the build, testDeploy, and deploy jobs so all actions/setup-node invocations
receive a robust version value.
The Test Deploy job runs 'firebase deploy --only functions:testFunction'
which executes the firebase.json 'predeploy' hooks. The hook still
called:
- yarn workspace functions lint
- yarn workspace functions test run
Both broke after the migration:
- The 'lint' script is now at the root only (functions/ no longer has
its own lint command).
- 'test run' was the jest CLI form ('jest run'); vitest's equivalent
is 'vitest run' which we exposed as the 'test:run' workspace
script.
Predeploy hooks now invoke:
- yarn lint (root oxlint over the workspace)
- yarn workspace functions run test:run (vitest run, scoped to functions)
- yarn workspace functions build (unchanged)
2 participants
This repo was already partially migrated (had oxlint, husky 9, lint-staged, vitest). Finishes the alignment with the rest of the rollout.
Drops
functions/) \u2014 replaced with oxfmt across the workspace.functions/workspace's eslint +@typescript-eslint/*+ jest \u2014 dropped entirely; the workspace now uses the root oxlint viayarn lint.shellcheckpackage (playbook trap setup contributor and styles #2: it's broken and corrupts actionlint output).mise.tomladds shellcheck plus actionlint and gitleaks as managed binaries.Adds
scripts/ensure-husky.mjsself-heal pattern + gitleaks integration in.husky/pre-commit. Drops the previousexec >/dev/ttyredirect that fails in non-interactive shells.typescript+vitest+unicorn+oxc(was just baseline correctness rules).tsgo --noEmitas defaulttypecheck;tsckept astypecheck:tscfallback.CI
Existing
main.ymlsteps re-ordered (Format / Lint / Typecheck / Test) andyarn format(which writes) replaced withyarn format:check(which doesn't).codecov-action@v4.0.1\u2192@v5.oxlint config tunes
typescript/no-extraneous-classoff \u2014 the codebase uses static-only classes as namespaces, idiomatic in this repo.no-await-in-loopoff \u2014 Firestore migrations and rate-limited HTTP loops are single-threaded by design.Caveats
13 pre-existing
typescript/no-explicit-anywarnings remain (mostly in firebase-functions handler signatures and json deserialisation). Per playbook trap #22 the lint script drops--deny-warnings.Verified locally
yarn lint\u2014 0 errors, 13 warningsyarn format:check\u2014 clean (73 files)yarn typecheck\u2014 cleanyarn test\u2014 7 / 7Summary by CodeRabbit
Documentation
Style