Skip to content

fixed the type of onPaginatedUpdate_experimental in simple_client#366

Open
UfukUstali wants to merge 1 commit intoget-convex:mainfrom
UfukUstali:fix/simple-client-pagination-type
Open

fixed the type of onPaginatedUpdate_experimental in simple_client#366
UfukUstali wants to merge 1 commit intoget-convex:mainfrom
UfukUstali:fix/simple-client-pagination-type

Conversation

@UfukUstali
Copy link
Copy Markdown

        let newValue;
        try {
          if (isPaginatedQuery) {
            newValue = this.paginatedClient.localQueryResultByToken(queryToken);
          } else {
            newValue = this.client.localQueryResultByToken(queryToken);
          }
        } catch (error) {
          if (!(error instanceof Error)) throw error;
          if (onError) {
            onError(
              error,
              "Second argument to onUpdate onError is reserved for later use",
            );
          } else {
            // Make some noise without unsubscribing or failing to call other callbacks.
            void Promise.reject(error);
          }
          continue;
        }
        callback(
          newValue,
          "Second argument to onUpdate callback is reserved for later use",
        );

https://github.com/get-convex/convex-backend/blob/main/npm-packages/convex/src/browser/simple_client.ts#L451-L474

return of localQueryResultByToken is PaginatedQueryResult not PaginationResult, this was a typo i think.


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

@ianmacartney
Copy link
Copy Markdown
Member

good catch!

@UfukUstali
Copy link
Copy Markdown
Author

UfukUstali commented Feb 21, 2026

i think the ci failed due to an unrelated rust issue.

also on an unrelated note, can i open another pr to make it more viable for community framework bindings to use this function rather then reinventing the wheel for pagination themselves
not much, just a few types and the same id generation from reacts experimental pagination hook

diff --git a/packages/convex-js/src/browser/simple_client.ts b/packages/convex-js/src/browser/simple_client.ts
index e614d42..5725005 100644
--- a/packages/convex-js/src/browser/simple_client.ts
+++ b/packages/convex-js/src/browser/simple_client.ts
@@ -8,9 +8,12 @@ import {
   UserIdentityAttributes,
 } from "./index.js";
 import {
+  BetterOmit,
+  Expand,
   FunctionArgs,
   FunctionReference,
   FunctionReturnType,
+  PaginationOptions,
   PaginationResult,
 } from "../server/index.js";
 import { getFunctionName } from "../server/api.js";
@@ -22,6 +25,7 @@ import {
 } from "./sync/paginated_query_client.js";
 import { PaginatedQueryResult } from "./sync/pagination.js";
 import { serializedQueryTokenIsPaginated } from "./sync/udf_path_utils.js";
+import { Value } from "../values/value.js";
 
 // In Node.js builds this points to a bundled WebSocket implementation. If no
 // WebSocket implementation is manually specified or globally available,
@@ -33,6 +37,17 @@ export function setDefaultWebSocketConstructor(ws: typeof WebSocket) {
   defaultWebSocketConstructor = ws;
 }
 
+type PaginatedQueryReference = FunctionReference<
+  "query",
+  "public",
+  { paginationOpts: PaginationOptions },
+  PaginationResult<any>
+>;
+
+type PaginatedQueryArgs<Query extends PaginatedQueryReference> = Expand<
+  BetterOmit<FunctionArgs<Query>, "paginationOpts">
+>;
+
 export type ConvexClientOptions = BaseConvexClientOptions & {
   /**
    * `disabled` makes onUpdate callback registration a no-op and actions,
@@ -260,13 +275,15 @@ export class ConvexClient {
    *
    * @return an {@link Unsubscribe} function to stop calling the callback.
    */
-  onPaginatedUpdate_experimental<Query extends FunctionReference<"query">>(
+  onPaginatedUpdate_experimental<Query extends PaginatedQueryReference>(
     query: Query,
-    args: FunctionArgs<Query>,
+    args: PaginatedQueryArgs<Query>,
     options: { initialNumItems: number },
-    callback: (result: PaginationResult<FunctionReturnType<Query>>) => unknown,
+    callback: (
+      result: PaginatedQueryResult<FunctionReturnType<Query>>,
+    ) => unknown,
     onError?: (e: Error) => unknown,
-  ): Unsubscribe<PaginatedQueryResult<FunctionReturnType<Query>[]>> {
+  ): Unsubscribe<PaginatedQueryResult<FunctionReturnType<Query>>> {
     if (this.disabled) {
       return this.createDisabledUnsubscribe<
         PaginatedQueryResult<FunctionReturnType<Query>>
@@ -275,13 +292,12 @@ export class ConvexClient {
 
     const paginationOptions = {
       initialNumItems: options.initialNumItems,
-      id: -1,
+      id: nextPaginationId(),
     };
 
     const { paginatedQueryToken, unsubscribe } = this.paginatedClient.subscribe(
       getFunctionName(query),
-      args,
-      // Simple client doesn't use IDs, there's no expectation that these queries remain separate.
+      args as Record<string, Value>,
       paginationOptions,
     );
 
@@ -311,7 +327,7 @@ export class ConvexClient {
     }
 
     const unsubscribeProps: RemoveCallSignature<
-      Unsubscribe<PaginatedQueryResult<FunctionReturnType<Query>[]>>
+      Unsubscribe<PaginatedQueryResult<FunctionReturnType<Query>>>
     > = {
       unsubscribe: () => {
         if (this.closed) {
@@ -322,11 +338,8 @@ export class ConvexClient {
         unsubscribe();
       },
       getCurrentValue: () => {
-        const result = this.paginatedClient.localQueryResult(
-          getFunctionName(query),
-          args,
-          paginationOptions,
-        );
+        const result =
+          this.paginatedClient.localQueryResultByToken(paginatedQueryToken);
         // cast to apply the specific function type
         return result as
           | PaginatedQueryResult<FunctionReturnType<Query>>
@@ -573,6 +586,13 @@ export class ConvexClient {
   }
 }
 
+let paginationId = 0;
+
+function nextPaginationId(): number {
+  paginationId++;
+  return paginationId;
+}
+
 // internal information tracked about each registered callback
 type QueryInfo = {
   callback: (result: any, meta: unknown) => unknown;

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