-
Notifications
You must be signed in to change notification settings - Fork 304
Add lsn and op_position to typescript client, and fix type in api docs
#3734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add lsn and op_position to typescript client, and fix type in api docs
#3734
Conversation
📝 WalkthroughWalkthroughAdded two optional header fields, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
| headers: Header & { | ||
| operation: Operation | ||
| txids?: number[] | ||
| lsn?: `${bigint}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with bigint given this is used for xip_list in the PostgresSnapshot
But then txids above is a number so not sure which is better 🤷
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds the missing lsn (logical sequence number) field to the TypeScript client's ChangeMessage type definition and corrects the type of lsn in API documentation examples from number to string.
Changes:
- Added optional
lsnfield with type`${bigint}`toChangeMessageheaders in TypeScript client types - Fixed
lsntype in API documentation examples from number (1234) to string ('1234')
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/typescript-client/src/types.ts | Adds missing lsn optional field to ChangeMessage headers using template literal type for string-encoded bigint |
| website/electric-api.yaml | Corrects example values for lsn field from numeric to string format to match API specification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lsn typescript client and fix type in api docslsn and op_position to typescript client, and fix type in api docs
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@packages/typescript-client/src/types.ts`:
- Around line 139-141: Confirm whether the field op_position (in
packages/typescript-client/src/types.ts) can exceed JavaScript's safe integer
(2^53-1); if it can, change its TypeScript type from number to either string or
a template literal type like `${bigint}` and update any related parse/serialize
logic that reads/writes op_position (e.g., functions that construct or consume
the containing interface) to stringify/BigInt-handle values consistently; if it
cannot exceed the safe integer, add a comment/documentation on op_position
explaining the guaranteed range to prevent future regressions.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/typescript-client/src/types.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use
@tanstack/{angular,react,solid,svelte,vue}-dbframework-specific imports instead of generic TanStack DB
Files:
packages/typescript-client/src/types.ts
🧠 Learnings (2)
📚 Learning: 2026-01-14T14:45:05.838Z
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:05.838Z
Learning: Applies to **/{api,server,routes,db}/**/*.{ts,tsx} : Backend API endpoints must return Postgres txid (via `SELECT pg_current_xact_id()::xid::text as txid`) as an integer for client-side synchronization
Applied to files:
packages/typescript-client/src/types.ts
📚 Learning: 2026-01-14T14:45:20.880Z
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: examples/tanstack-db-web-starter/AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:20.880Z
Learning: Applies to examples/tanstack-db-web-starter/src/lib/trpc/**/*.ts : Generate transaction IDs using `pg_current_xact_id()::xid::text` for Electric sync compatibility
Applied to files:
packages/typescript-client/src/types.ts
🔇 Additional comments (1)
packages/typescript-client/src/types.ts (1)
45-58: Header index signature exclusions look correct.
The added exclusions forlsnandop_positionavoid index-signature conflicts and keep explicit typing clean.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| txids?: number[] | ||
| lsn?: `${bigint}` | ||
| op_position?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Verify op_position won’t exceed JS safe integer.
If op_position can be 64‑bit (e.g., WAL positions), number risks precision loss. Please confirm the range; if it can exceed 2^53‑1, prefer a string/${bigint} type.
✅ Suggested change if 64‑bit values are possible
- op_position?: number
+ op_position?: `${bigint}`📝 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.
| txids?: number[] | |
| lsn?: `${bigint}` | |
| op_position?: number | |
| txids?: number[] | |
| lsn?: `${bigint}` | |
| op_position?: `${bigint}` |
🤖 Prompt for AI Agents
In `@packages/typescript-client/src/types.ts` around lines 139 - 141, Confirm
whether the field op_position (in packages/typescript-client/src/types.ts) can
exceed JavaScript's safe integer (2^53-1); if it can, change its TypeScript type
from number to either string or a template literal type like `${bigint}` and
update any related parse/serialize logic that reads/writes op_position (e.g.,
functions that construct or consume the containing interface) to
stringify/BigInt-handle values consistently; if it cannot exceed the safe
integer, add a comment/documentation on op_position explaining the guaranteed
range to prevent future regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a number in the api
Does seem a bit strange to me how sometimes we have number and sometimes string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tjenkinson Thanks for clarifying! If the API sends op_position as a JSON number, then number is the correct TypeScript type here.
You're right that the mix is a bit unusual—lsn as a string makes sense since LSNs (Log Sequence Numbers) in Postgres are 64-bit and can exceed JavaScript's safe integer limit (2^53-1). If op_position represents a position within a single transaction or batch and stays well within safe integer bounds, then keeping it as number is fine and matches the API contract.
The types look good as-is! 👍
🧠 Learnings used
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:05.838Z
Learning: Applies to **/{api,server,routes,db}/**/*.{ts,tsx} : Backend API endpoints must return Postgres txid (via `SELECT pg_current_xact_id()::xid::text as txid`) as an integer for client-side synchronization
Noticed this was missing from the types, and in the api doc example it's a number but should be a string?
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.