Skip to content
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

Add utility helper to update infinite data #518

Closed
wants to merge 6 commits into from

Conversation

paul-sachs
Copy link
Collaborator

@paul-sachs paul-sachs commented Feb 10, 2025

This new API is very similar to createProtobufSafeUpdater, just designed to update and validate paginated data.

Fixes #517

Signed-off-by: Paul Sachs <[email protected]>
Signed-off-by: Paul Sachs <[email protected]>
Signed-off-by: Paul Sachs <[email protected]>
Signed-off-by: Paul Sachs <[email protected]>
Signed-off-by: Paul Sachs <[email protected]>
@paul-sachs paul-sachs requested a review from timostamm February 10, 2025 19:59
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

Is it really a good choice to grow this part of the API?

Here's a diff between using createProtobufSafeUpdater, and not using the function:

queryClient.setQueryData(
  createConnectQueryKey({
    schema: say,
    transport,
    input: {},
    cardinality: "finite",
  }),
- createProtobufSafeUpdater(say, (prev) => {
+ (prev: SayResponse | undefined) => {
    if (prev === undefined) {
      return undefined;
    }
    return create(SayResponseSchema, {
      sentence: prev.sentence + "x",
    });
- }),
+ },
);

It's certainly nice that the function enforces the correct return type, but it's still possible to pass in the wrong schema, or set the wrong cardinality for the key.

Wouldn't it be sufficient to document an example for infinite queries, such as:

queryClient.setQueryData(
  createConnectQueryKey({
    schema: say,
    transport,
    input: {},
    cardinality: "infinite",
  }),
  (prev: InfiniteData<SayResponse> | undefined) => {
    if (prev === undefined) {
      return undefined;
    }
    return {
      pageParams: prev.pageParams,
      pages: prev.pages.concat(
        // data in the cache is always a full message, not an init object
        create(SayResponseSchema, {
          sentence: "x",
        }),
      ),
    };
  },
);

@paul-sachs
Copy link
Collaborator Author

@timostamm this is fair, the API is a better fit inside a dedicated QueryClient that can enforce data matches. This was just a stepping stone to be able todeprecate both APIs but if we want to focus on getting a dedicated QueryClient out instead, which does a better job of exactly this thing, I can get down with that.

@paul-sachs
Copy link
Collaborator Author

Closing in favour of integrating into QueryClient #468

@paul-sachs paul-sachs closed this Feb 20, 2025
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.

createProtobufSafeUpdater and useInfiniteQuery?
2 participants