Skip to content

Conversation

@Nicolapps
Copy link
Member

@Nicolapps Nicolapps commented Dec 17, 2025


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Summary by CodeRabbit

  • Bug Fixes

    • Resolved type checking issues in row-level security and triggers functionality when using table name arguments with database operations.
  • Chores

    • Updated Convex dependency to version 1.31.2.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 17, 2025

Walkthrough

Updated Convex dependency from ^1.31.0 to ^1.31.2 and adjusted public type signatures in row-level security and trigger APIs. The changes swap how the NonUnion constraint is applied between table name and ID parameters across multiple method overloads without altering runtime behavior.

Changes

Cohort / File(s) Summary
Dependency Update
package.json
Updated convex dependency from ^1.31.0 to ^1.31.2
Changelog Entry
packages/convex-helpers/CHANGELOG.md
Added version 0.1.109 entry documenting a type issue fix for Triggers/row-level security in ctx.db APIs with table name arguments, referencing convex@1.31.2
Type Signature Adjustments
packages/convex-helpers/server/rowLevelSecurity.ts, packages/convex-helpers/server/triggers.ts
Refactored public method overloads in WrapReader, WrapWriter, DatabaseWriterWithTriggers, and writerWithTriggers to replace NonUnion<TableName> with TableName for table parameters and swap id parameters from GenericId<TableName> to GenericId<NonUnion<TableName>> across get, patch, replace, delete, and delete_ methods

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • Type-only changes following a homogeneous pattern across multiple overloads with no runtime logic modifications
  • Primary concern: verify public API type changes do not introduce downstream breaking changes or type misalignment with consumers of WrapReader, WrapWriter, DatabaseWriterWithTriggers, and writerWithTriggers

Possibly related PRs

  • PR #873: Modifies the same rowLevelSecurity/triggers API surface with generic type adjustments around TableName, NonUnion, and id signatures
  • PR #880: Updates the same files (rowLevelSecurity.ts, triggers.ts) with identical TypeScript overload adjustments and convex dependency bump
  • PR #881: Modifies triggers.ts alongside this PR; while this PR tightens overload type signatures, the related PR addresses runtime trigger iteration/handling

Suggested reviewers

  • ianmacartney

Poem

🐰 Generics dance and types align,
NonUnion swaps in line,
Table names now free to roam,
IDs find their proper home,
APIs gleam with types so tight—
Row-level security shines bright!

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 summarizes the main change: updating type signatures in row-level security and triggers modules to align with convex@1.31.2 dependency update.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nicolas/fix-types

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffa7946 and 5313df4.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • package.json (1 hunks)
  • packages/convex-helpers/CHANGELOG.md (1 hunks)
  • packages/convex-helpers/server/rowLevelSecurity.ts (5 hunks)
  • packages/convex-helpers/server/triggers.ts (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When modifying complex TypeScript types, run npm run typecheck in the repository root to ensure types are correct, rather than running tsc manually

Files:

  • packages/convex-helpers/server/rowLevelSecurity.ts
  • packages/convex-helpers/server/triggers.ts
🧬 Code graph analysis (1)
packages/convex-helpers/server/triggers.ts (1)
convex/_generated/dataModel.d.ts (1)
  • DataModel (60-60)
🔇 Additional comments (6)
package.json (1)

30-30: Verify if devDependencies convex version should also be updated.

The main dependencies now uses ^1.31.2, but devDependencies (line 56) still pins convex to 1.31.0. This version mismatch could cause type inconsistencies during development and testing if the type signatures differ between versions.

Is this intentional for backward compatibility testing, or should the devDependency also be updated to 1.31.2?

packages/convex-helpers/server/rowLevelSecurity.ts (2)

239-242: Type signature update looks correct.

The change from NonUnion<TableName> on the table parameter to TableName, combined with moving NonUnion to the id parameter (GenericId<NonUnion<TableName>>), aligns with the convex@1.31.2 API. This pattern is consistently applied across all method overloads in this file.


358-361: Consistent type updates across all WrapWriter methods.

All method overloads (patch, replace, delete, get) follow the same pattern of moving NonUnion from the table parameter to the id parameter. The changes are consistent with the WrapReader.get update and align with convex@1.31.2 expectations.

Also applies to: 376-379, 394-397, 407-410

packages/convex-helpers/server/triggers.ts (2)

173-176: Type signatures in DatabaseWriterWithTriggers updated consistently.

All method overloads in the deprecated DatabaseWriterWithTriggers class follow the same pattern as rowLevelSecurity.ts, ensuring type consistency for users who haven't migrated to writerWithTriggers yet.

Also applies to: 182-185, 200-203, 226-229


254-257: Type signatures in writerWithTriggers updated consistently.

The patch, replace, and delete_ function overloads in writerWithTriggers all correctly apply the same type pattern (table: TableName, id: GenericId<NonUnion<TableName>>), maintaining consistency with both the deprecated class and the rowLevelSecurity.ts changes.

Also applies to: 295-298, 336-339

packages/convex-helpers/CHANGELOG.md (1)

3-8: Changelog entry is clear and well-documented.

The entry appropriately describes the fix, specifies the affected APIs (ctx.db with table name arguments), and references the upstream convex@1.31.2 changelog for users who want more context.


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.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 17, 2025

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/convex-helpers@884

commit: 5313df4

@Nicolapps Nicolapps merged commit 1f71559 into main Dec 18, 2025
5 checks passed
@Nicolapps Nicolapps deleted the nicolas/fix-types branch December 18, 2025 21:04
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