Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
🦋 Changeset detectedLatest commit: b0070de The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a pnpm version catalog and replaces many explicit dependency ranges with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Greptile SummaryThis PR upgrades the SafeQL ESLint plugin ecosystem from ESLint 9 → 10, Node 20 → 24, and Key changes:
Confidence Score: 4/5
Important Files Changed
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
20-23:⚠️ Potential issue | 🟠 MajorAdd Node version matrix to CI to test all declared support versions.
The workflow tests only Node 24 (line 42) but
package.jsondeclares support for^20.19.0 || ^22.13.0 || >=24. Versions 20.x and 22.x are untested in CI, creating a coverage gap.Proposed fix
strategy: matrix: + node-version: [20.19.0, 22.13.0, 24] postgres-version: [16, 17] fail-fast: false ... - uses: actions/setup-node@v4 with: - node-version: 24 + node-version: ${{ matrix.node-version }} cache: "pnpm"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 20 - 23, CI only tests Node 24 while package.json supports ^20, ^22 and >=24; update the GitHub Actions matrix in .github/workflows/ci.yml under the strategy.matrix block to include a node-version axis (e.g., node-version: [20,22,24]) alongside the existing postgres-version so every supported Node release is tested; ensure matrix combinations or an explicit include list covers the desired pairings and update any job uses of the Node version variable to reference the new matrix key.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
19-19: Consider using24.xfor consistency.The previous version used
20.xformat. Using24.xwould maintain consistency and allow automatic minor/patch updates.Suggested change
- node-version: 24 + node-version: 24.x🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml at line 19, Update the GitHub Actions workflow node version specification: locate the node-version key in the release workflow (the line currently set to "24") and change its value to "24.x" so it matches the previous "20.x" style, allowing automatic minor/patch updates and consistent version formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/eslint-plugin/package.json`:
- Around line 79-81: Add an explicit eslint peer dependency in package.json so
host projects get compatibility warnings: update the "peerDependencies" object
(the same block currently containing "@typescript-eslint/utils" and
"libpg-query") to include "eslint" with the semver range your plugin supports
(e.g. a major-compatible range like ">=8 <10" or "^8.0.0" if you only support
ESLint v8); ensure the new peer entry is present alongside the existing peers
(you may keep eslint in devDependencies for testing but do not rely on it as the
only declaration).
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 20-23: CI only tests Node 24 while package.json supports ^20, ^22
and >=24; update the GitHub Actions matrix in .github/workflows/ci.yml under the
strategy.matrix block to include a node-version axis (e.g., node-version:
[20,22,24]) alongside the existing postgres-version so every supported Node
release is tested; ensure matrix combinations or an explicit include list covers
the desired pairings and update any job uses of the Node version variable to
reference the new matrix key.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Line 19: Update the GitHub Actions workflow node version specification: locate
the node-version key in the release workflow (the line currently set to "24")
and change its value to "24.x" so it matches the previous "20.x" style, allowing
automatic minor/patch updates and consistent version formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6419cd53-d563-4f71-aae8-3c8378d1f22d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.changeset/eslint-10-major.md.github/workflows/ci.yml.github/workflows/release.yml.nvmrcdemos/basic-flat-config/package.jsondemos/basic-migrations-raw/package.jsondemos/basic-transform-type/package.jsondemos/basic/package.jsondemos/big-project/package.jsondemos/config-file-flat-config/package.jsondemos/from-config-file/package.jsondemos/multi-connections/package.jsondemos/playground/package.jsondemos/plugin-aws-iam/package.jsondemos/plugin-aws-iam/scripts/verify-plugin-error.shdemos/postgresjs-custom-types/package.jsondemos/postgresjs-demo/package.jsondemos/prisma/package.jsondemos/typeorm/package.jsondemos/vercel-postgres/package.jsonpackage.jsonpackages/ast-types/package.jsonpackages/eslint-plugin/package.jsonpackages/eslint-plugin/src/rules/check-sql.utils.tspackages/generate/package.jsonpackages/plugin-utils/package.jsonpackages/shared/package.jsonpackages/sql-tag/package.jsonpackages/test-utils/package.jsonpackages/zod-annotator/package.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/sql-tag/src/sql-tag.ts (1)
30-31: Avoid recomputingthis.valuesinside the reducer.Line 31 reads
this.values.lengthon each iteration; with atransform, that remaps all values repeatedly just to get length. Cache length once (or usethis.rawValues.length) beforereduce.♻️ Proposed refactor
get text() { + const valuesLength = this.rawValues.length; return this.template.reduce( - (acc, part, i) => acc + part + (i === this.values.length ? "" : `$${i + 1}`), + (acc, part, i) => acc + part + (i === valuesLength ? "" : `$${i + 1}`), "", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sql-tag/src/sql-tag.ts` around lines 30 - 31, The reducer currently calls this.values.length on every iteration causing repeated remapping; fix by caching the length before the reduce (e.g., const valsLen = this.values.length or use this.rawValues.length) and reference that cached variable inside the reducer when comparing i === valsLen; update the method that builds the SQL string (the reduce on this.template) to use the cached length to avoid recomputing this.values each iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/sql-tag/src/sql-tag.ts`:
- Around line 30-31: The reducer currently calls this.values.length on every
iteration causing repeated remapping; fix by caching the length before the
reduce (e.g., const valsLen = this.values.length or use this.rawValues.length)
and reference that cached variable inside the reducer when comparing i ===
valsLen; update the method that builds the SQL string (the reduce on
this.template) to use the cached length to avoid recomputing this.values each
iteration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2dab362f-49ec-48e2-b1c6-15124bbeb121
📒 Files selected for processing (5)
package.jsonpackages/eslint-plugin/package.jsonpackages/plugin-utils/package.jsonpackages/sql-tag/src/sql-tag.tspackages/zod-annotator/package.json
✅ Files skipped from review due to trivial changes (3)
- packages/plugin-utils/package.json
- package.json
- packages/eslint-plugin/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod-annotator/package.json
|
@Newbie012 would you be able to publish a new release with this? I would like to upgrade to ESLint 10 in our projects using SafeQL. Also, I guess this PR should have been marked as the following? Closes #450 |
Summary by CodeRabbit
New Features
Chores
Bug Fixes