fix: ensure remote function queries update when using schema transfor…#15738
fix: ensure remote function queries update when using schema transfor…#15738machadinhos wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: bd5c924 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dummdidumm
left a comment
There was a problem hiding this comment.
This is great, thank you! For the test, add a new page in kit/test/apps/async/src/routes/remote/form and add a corresponding test in the related test/test.js. cc @elliott-with-the-longest-name-on-github for a second pair of eyes since you did the original work here
|
couldn't check if the test was passing because when I ran the tests I only saw errors from other tests even if I introduced an error by hand. |
|
Opened an alternative PR. This is actually a good PR, but while reviewing it I realized our approach is more fundamentally flawed in a way that patching like this can't fix: #15739 |
|
(also your commits are still included in the other PR, so you'll get credit for your work 😁) |
|
Thank you so much! |
Alternative to #15738 Closes #15696 I realized while reviewing that there's a fundamental problem: If the query's validator lossily changes the input, there's no way to map the validated input back to the correct argument. Consider a validator that calls `Math.floor` on its input number, then consider the following: ```ts // from the client, we request `query(1.2)` and `query(1.8)`, // but both `arg`s are `1` for await (const arg of requested(my_query, 5)) { // there is no way to map `1` back to the original inputs; // only 1.8 would actually be refreshed because it wrote // to the `validated` cache last void my_query(arg).refresh(); } ``` This realization in hand, I further realized the only way to make this work is to somehow preserve the mapping of input arguments to validated arguments. The best way to do that is to return a bound instance of the query from `requested`: ```ts for await (const { arg, query } of requested(my_query, 5)) { void query.refresh(); } ``` Problem solved; the `query` internally maintains the map of input argument => validated argument. The `arg` is still the validated argument in case you need to do something like `if (arg.id === id)`. --------- Co-authored-by: machadinhos <pbmachado2002@gmail.com> Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Closes #15696
If a remote function query has a validator that returns a value different from its original input, the generated cache key will use the validator’s output instead of the original input when refreshing the query. This can lead to queries not being refreshed correctly even when the same argument is passed.
I was not able to test these changes. I followed the CONTRIBUTING.md steps to add
pnpm.overridesto thepackage.jsonof a test app but I got this error when trying to run it:This is my first PR here so please be gentle :D
Thank you!
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits