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

V2: Update createProtobufSafeUpdater #447

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

timostamm
Copy link
Member

@timostamm timostamm commented Oct 8, 2024

Closes #446. The signature of the provided updater function is changed to return a message instead of a message init shape.

Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
Comment on lines +59 to +60
| undefined
| ((prev?: MessageShape<O>) => MessageShape<O> | undefined);
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding undefined on line 59 supports a use-case with QueryClient.setQueryData:

If the value is undefined, the query data is not updated.

This means the following example compiles and works as expected (query data is not updated):

const queryKey = createConnectQueryKey({
  schema: ElizaService.method.say,
  input: { sentence: "hi" },
  transport: myTransport,
});

const newData: SayResponse | undefined = undefined;

const updater = createProtobufSafeUpdater(
  ElizaService.method.say,
  newData,
);

queryClient.setQueryData(queryKey, updater);

Comment on lines +120 to +128
const safeUpdater = createProtobufSafeUpdater(schema, (prev) => {
if (prev === undefined) {
return undefined;
}
return {
...prev,
int32Field: 999,
};
});
Copy link
Member Author

Choose a reason for hiding this comment

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

We have to explicitly handle the undefined case now. In this example, we do not update query data if it does not exist.

If the goal is to create query data if none exists, the function could be updated to:

const safeUpdater = createProtobufSafeUpdater(schema, (prev) => {
  prev ??= create(schema.output);
  return {
    ...prev,
    int32Field: 999,
  };
});

We could move this logic into createProtobufSafeUpdater, and always provide data to the updater function. But then it isn't possible to update only existing query data. I'm not sure what's idiomatic here. WDYT, @paul-sachs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this new version is more idiomatic to react-query so I'm good with this updated behaviour.

@timostamm timostamm requested a review from paul-sachs October 8, 2024 11:42
@timostamm timostamm changed the title Update createProtobufSafeUpdater V2: Update createProtobufSafeUpdater Oct 8, 2024
@timostamm timostamm merged commit bc83dd0 into v2 Oct 8, 2024
8 checks passed
@timostamm timostamm deleted the tstamm/v2/Update-createProtobufSafeUpdater branch October 8, 2024 13:15
@timostamm timostamm mentioned this pull request Nov 19, 2024
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