Skip to content

chore!: upgrade to ESLint 10 and Node.js 24#454

Merged
Newbie012 merged 5 commits intomainfrom
eslint-10
Mar 21, 2026
Merged

chore!: upgrade to ESLint 10 and Node.js 24#454
Newbie012 merged 5 commits intomainfrom
eslint-10

Conversation

@Newbie012
Copy link
Copy Markdown
Collaborator

@Newbie012 Newbie012 commented Mar 21, 2026

Summary by CodeRabbit

  • New Features

    • Added support for ESLint 10 (major release bumps recorded; includes breaking note)
  • Chores

    • Raised Node.js requirement to >=24 and updated CI/release runtimes
    • Introduced a centralized dependency version catalog and replaced many package version specifiers
    • Made demo AWS verification deterministic for consistent failure reproduction
    • Broadened TypeScript lib target to ESNext
  • Bug Fixes

    • Adjusted SQL parameter placeholder numbering (may affect parameter indexing)

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
safeql Ignored Ignored Preview Mar 21, 2026 8:47pm

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 21, 2026

🦋 Changeset detected

Latest commit: b0070de

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@ts-safeql/eslint-plugin Major
@ts-safeql/plugin-utils Major
@ts-safeql/zod-annotator Major
@ts-safeql-demos/basic-flat-config Patch
@ts-safeql-demos/basic-migrations-raw Patch
@ts-safeql-demos/basic-transform-type Patch
@ts-safeql-demos/basic Patch
@ts-safeql-demos/big-project Patch
@ts-safeql-demos/config-file-flat-config Patch
@ts-safeql-demos/from-config-file Patch
@ts-safeql-demos/multi-connections Patch
@ts-safeql-demos/playground Patch
@ts-safeql-demos/plugin-aws-iam Patch
@ts-safeql-demos/postgresjs-custom-types Patch
@ts-safeql-demos/postgresjs-demo Patch
@ts-safeql-demos/vercel-postgres Patch
@ts-safeql/connection-manager Patch
@ts-safeql/plugin-auth-aws Patch
@ts-safeql/generate Major
@ts-safeql/shared Major
@ts-safeql/sql-ast Major
@ts-safeql/test-utils Patch

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 21, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7699da3d-7b30-42c8-9ec5-2dd65e4dd73c

📥 Commits

Reviewing files that changed from the base of the PR and between 903cc8b and b0070de.

📒 Files selected for processing (1)
  • tsconfig.node.json
✅ Files skipped from review due to trivial changes (1)
  • tsconfig.node.json

📝 Walkthrough

Walkthrough

Adds a pnpm version catalog and replaces many explicit dependency ranges with catalog: placeholders, bumps Node targets to v24, adds an ESLint-10 changeset, tweaks ESLint filename access, adjusts SqlTag parameter indexing, and makes an AWS demo verification script deterministic.

Changes

Cohort / File(s) Summary
Changeset
.changeset/eslint-10-major.md
Adds a changeset marking a major release for ESLint-related packages and declares ESLint 10 support (BREAKING).
Node runtime & CI
.nvmrc, .github/workflows/ci.yml, .github/workflows/release.yml, package.json
Bumps Node targets to v24 / >=24 and fixes GH Actions Node versions to 24.
Version catalog
pnpm-workspace.yaml
Adds a top-level catalog block centralizing versions for tooling and select packages.
Workspace dependency spec updates
package.json, demos/*/package.json, packages/*/package.json, demos/shared/package.json
Replaces many explicit semver ranges with the "catalog:" placeholder across demos and packages; manifests may have gained/removed trailing newlines. Review dependency resolution changes.
ESLint rule filename access
packages/eslint-plugin/src/rules/check-sql.utils.ts
shouldLintFile now reads filename from params.filename instead of calling params.getFilename().
SqlTag parameter indexing
packages/sql-tag/src/sql-tag.ts
Placeholder generation changed from using pre-incremented ++i to i + 1, altering parameter numbering in emitted SQL.
AWS demo script deterministic env
demos/plugin-aws-iam/scripts/verify-plugin-error.sh
Exports AWS_EC2_METADATA_DISABLED=true, local DB env vars, SAFEQL_AWS_REGION, and a missing SAFEQL_AWS_PROFILE to force deterministic plugin failure conditions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

"I nibble bytes and hop with glee,
Catalogued versions snug with me,
Node climbs up and linters cheer,
Placeholders count and scripts are clear,
A rabbit stamps — release is near." 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main changes: upgrading to ESLint 10 and Node.js 24, which are the core objectives reflected throughout the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch eslint-10

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 21, 2026

Greptile Summary

This PR upgrades the SafeQL ESLint plugin ecosystem from ESLint 9 → 10, Node 20 → 24, and typescript-eslint 8.288.57. The only functional source code change is replacing the removed context.getFilename() call with context.filename in check-sql.utils.ts — everything else is dependency version bumps, peer-dep range adjustments, a CI fix, and a test-script hardening.

Key changes:

  • All demo and package package.json files bump eslint to ^10.0.3, @eslint/js to ^10.0.1, and typescript-eslint to ^8.57.1.
  • The three published packages (eslint-plugin, plugin-utils, zod-annotator) narrow their @typescript-eslint/utils peer dep from >=8.0.0 to ^8.57.1 and are correctly marked as a major bump in the changeset.
  • .github/workflows/ci.yml fixes a latent bug: matrix.node-version was referenced in the setup-node step but was never defined in the matrix (only postgres-version was), so the old workflow silently used whatever Node version the GitHub Actions runner provided. It is now explicitly set to 24.
  • engines in root package.json is widened to ^20.19.0 || ^22.13.0 || >=24, but CI only tests Node 24 — a minor coverage gap.
  • demos/plugin-aws-iam/scripts/verify-plugin-error.sh adds AWS_EC2_METADATA_DISABLED=true and forces deterministic SafeQL env vars, preventing flaky runs that depend on EC2 metadata lookups or developer-local config.

Confidence Score: 4/5

  • PR is safe to merge; one minor documentation/CI coverage gap remains between the declared engines Node versions and what CI actually tests.
  • The single source code change (getFilename()filename) is correct and is the canonical ESLint 10 migration. All version bumps are consistent across the monorepo, the lockfile is updated, and a proper major changeset is present. The only non-blocking concerns are: (1) Node 20/22 are advertised in engines but not CI-tested, and (2) the peer dep floor of ^8.57.1 is stricter than the actual API surface requires, which could surprise users upgrading from older typescript-eslint 8.x.
  • packages/eslint-plugin/package.json, packages/plugin-utils/package.json, and packages/zod-annotator/package.json (peer dep narrowing); .github/workflows/ci.yml (Node version test matrix).

Important Files Changed

Filename Overview
packages/eslint-plugin/src/rules/check-sql.utils.ts Migrates context.getFilename() (removed in ESLint 10) to context.filename — the one functional code change in the PR; correct and minimal.
packages/eslint-plugin/package.json Bumps ESLint/typescript-eslint devDeps; narrows @typescript-eslint/utils peer dep from >=8.0.0 to ^8.57.1, intentionally dropping support for older 8.x versions as part of the major bump.
packages/plugin-utils/package.json Same peer dep narrowing as eslint-plugin: >=8.0.0^8.57.1 for @typescript-eslint/utils; intentional breaking change.
packages/zod-annotator/package.json Same peer dep narrowing as eslint-plugin and plugin-utils; intentional breaking change.
.github/workflows/ci.yml Fixes a latent bug where ${{ matrix.node-version }} was referenced but never defined in the matrix (only postgres-version was); now hardcoded to Node 24. CI no longer cross-tests the Node 20/22 versions still listed in engines.
.github/workflows/release.yml Node version updated from 20.x to 24 for the release workflow; straightforward and correct.
demos/plugin-aws-iam/scripts/verify-plugin-error.sh Adds AWS_EC2_METADATA_DISABLED=true and deterministic SAFEQL/AWS env vars so the script no longer relies on developer-local config and avoids slow EC2 metadata lookups in CI.
package.json Expands the engines.node field from "20.x" to `"^20.19.0
.changeset/eslint-10-major.md Correctly records the major-version bump for the three published packages that narrow their peer dependency on @typescript-eslint/utils.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["User project: ESLint 9 or 10"] --> B{"@typescript-eslint/utils version?"}
    B -- "< 8.57.1" --> C["❌ Peer dep error\n(new SafeQL major)"]
    B -- "^8.57.1" --> D["✅ Compatible with new SafeQL major"]

    D --> E{"Which ESLint version?"}
    E -- "ESLint 9" --> F["context.filename ✅\n(deprecated getFilename removed in v10)"]
    E -- "ESLint 10" --> G["context.filename ✅\n(only valid API in v10)"]

    F --> H["SafeQL rule runs"]
    G --> H

    subgraph "CI Coverage"
        I["Node 24 ✅"]
        J["Node 22 🚫 (declared in engines but untested)"]
        K["Node 20 🚫 (declared in engines but untested)"]
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 42

Comment:
**Node 20/22 not tested despite being in `engines`**

The root `package.json` declares `"node": "^20.19.0 || ^22.13.0 || >=24"` as supported versions, but CI now only tests against Node 24. The previous workflow line `node-version: ${{ matrix.node-version }}` was in fact a latent bug — `matrix.node-version` was never defined in the `strategy.matrix` block (only `postgres-version` was), so it evaluated to an empty string and `actions/setup-node` likely fell back to whatever Node version the runner provided.

Now that CI is intentionally pinned to 24, consider either adding a multi-version node matrix or adjusting the `engines` field to `>=24` to avoid a documentation mismatch where Node 20/22 are advertised as supported but not tested.

```suggestion
          node-version: ${{ matrix.node-version }}
```
with a corresponding matrix entry:
```yaml
      matrix:
        postgres-version: [16, 17]
        node-version: [20, 22, 24]
```
Or, if only Node 24 is the intent going forward, simply update `engines` in `package.json` to `">=24"` so the declared support matches what is verified.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/eslint-plugin/package.json
Line: 80

Comment:
**Peer dep narrowing drops all `@typescript-eslint/utils` 8.0–8.56 users**

The range changed from `>=8.0.0` to `^8.57.1`, meaning anyone currently on any `@typescript-eslint/utils` release between `8.0.0` and `8.56.x` will get an unsatisfied-peer-dependency error when updating to this major version of SafeQL — even if their existing setup works fine with ESLint 9.

Since `context.filename` (the only API change in source code) was already available in ESLint 9, the plugin itself would function correctly with older 8.x typescript-eslint versions. Consider using `^8.0.0` and gating only what actually requires `8.57.1`, or document the minimum required version more explicitly in the changeset/migration notes.

The same narrowing applies to `packages/plugin-utils/package.json:43` and `packages/zod-annotator/package.json:69`.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "chore: upgrade to ES..."

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟠 Major

Add Node version matrix to CI to test all declared support versions.

The workflow tests only Node 24 (line 42) but package.json declares 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 using 24.x for consistency.

The previous version used 20.x format. Using 24.x would 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

📥 Commits

Reviewing files that changed from the base of the PR and between acd33af and 2e82acf.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (30)
  • .changeset/eslint-10-major.md
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • .nvmrc
  • demos/basic-flat-config/package.json
  • demos/basic-migrations-raw/package.json
  • demos/basic-transform-type/package.json
  • demos/basic/package.json
  • demos/big-project/package.json
  • demos/config-file-flat-config/package.json
  • demos/from-config-file/package.json
  • demos/multi-connections/package.json
  • demos/playground/package.json
  • demos/plugin-aws-iam/package.json
  • demos/plugin-aws-iam/scripts/verify-plugin-error.sh
  • demos/postgresjs-custom-types/package.json
  • demos/postgresjs-demo/package.json
  • demos/prisma/package.json
  • demos/typeorm/package.json
  • demos/vercel-postgres/package.json
  • package.json
  • packages/ast-types/package.json
  • packages/eslint-plugin/package.json
  • packages/eslint-plugin/src/rules/check-sql.utils.ts
  • packages/generate/package.json
  • packages/plugin-utils/package.json
  • packages/shared/package.json
  • packages/sql-tag/package.json
  • packages/test-utils/package.json
  • packages/zod-annotator/package.json

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/sql-tag/src/sql-tag.ts (1)

30-31: Avoid recomputing this.values inside the reducer.

Line 31 reads this.values.length on each iteration; with a transform, that remaps all values repeatedly just to get length. Cache length once (or use this.rawValues.length) before reduce.

♻️ 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2e82acf and 8d4a476.

📒 Files selected for processing (5)
  • package.json
  • packages/eslint-plugin/package.json
  • packages/plugin-utils/package.json
  • packages/sql-tag/src/sql-tag.ts
  • packages/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 Newbie012 merged commit 00b9904 into main Mar 21, 2026
5 checks passed
@Newbie012 Newbie012 deleted the eslint-10 branch March 21, 2026 20:49
@karlhorky
Copy link
Copy Markdown
Collaborator

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants