fix(solid-query): hydrate preloaded data correctly #5775

marbemac · 2023-07-24T15:50:13Z

Copied from #5775, rebased on beta branch.

Make sure that on the server we always resolve the resource data to a hydrate-able value, otherwise solid will silently call onHydrated on the client with an empty object which can cause a variety of problems.

The specific code branch we were missing was when createQuery is called and observer.getOptimisticResult(defaultedOptions) returns an already loaded query (if, for example, preloadQuery was called prior to the components mounting - for example with router data loaders).

As always, these SSR / hydration cases are difficult to test, so I added an example to the solid-start-streaming to manually verify for now. This example simply calls preloadQuery on the server, and then renders a component that uses that same query.

Before 1

This is with deferStream: false on the user query - note how the content ends up getting rendered twice, and in the dev console you can see that onHydrated is called with an empty object. Things work as expected after the changes in this PR.

Before 2

This is with deferStream: true on the user query - note how it ends up calling the user query on the server AND on the client (see logs in dev console), and in the dev console you can see that onHydrated is called with an empty object (which causes the client to think that it needs to refetch, hence the client call).

After

Signed-off-by: marbemac <marbemac@gmail.com> # Conflicts: # packages/solid-query/src/createBaseQuery.ts

Signed-off-by: marbemac <marbemac@gmail.com>

Signed-off-by: marbemac <marbemac@gmail.com> # Conflicts: # examples/solid/solid-start-streaming/package.json # pnpm-lock.yaml

Signed-off-by: marbemac <marbemac@gmail.com>

vercel · 2023-07-24T15:50:17Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
query	⬜️ Ignored (Inspect)			Jul 25, 2023 3:03pm

marbemac · 2023-07-24T15:50:40Z

packages/solid-query/package.json

    "test:types": "tsc",
    "test:lib": "vitest run --coverage",
-    "test:lib:dev": "pnpm run test:lib --watch",
+    "test:lib:dev": "vitest watch --coverage",


vitest --watch is a thing, vitest run --watch isn't - they have a dedicated watch command instead.

packages/solid-query/src/createBaseQuery.ts

marbemac · 2023-07-24T15:51:05Z

packages/solid-query/src/createBaseQuery.ts

-          if (process.env['NODE_ENV'] === 'development') {
-            console.error(unwrappedResult.error)
-          }


Wasn't sure if this log is supposed to be here or an accidental leftover - lmk if it should stay and I can add it back.

We added a log in there because of this discussion https://discord.com/channels/719702312431386674/1072140965910949929

Please let me know if you have a better way to resolve/fix this 😅

marbemac · 2023-07-24T15:51:19Z

packages/solid-query/src/createBaseQuery.ts

-        if (unwrappedResult.isSuccess) {
-          // Use of any here is fine
-          // We cannot include refetch since it is not serializable
-          resolve(unwrappedResult as any)
+        } else {
+          resolve(unwrappedResult)


IMHO we should always fall back to calling resolve(). I ran into an issue where neither reject nor resolve were called, resulting in a hung promise (and this was on the server, resulting in the entire page being hung). LMK if there's a reason to sometimes not resolve the promise.

If we dont call reject here. How do we throw an error to the ErrorBoundary on the server?

Well it's still calling reject like it was before. There was just an additional check where if we decided not to reject, we would only call resolve under certain conditions. Thus there was a scenario where neither reject NOR resolve would get called, leaving a hanging promise. Does that make sense?

marbemac · 2023-07-24T15:51:30Z

packages/solid-query/src/createBaseQuery.ts

-          resolve(state)
+          const query = observer.getCurrentQuery()
+          resolve(hydrateableObserverResult(query, state))


Here is the code path that wasn't covered - this is triggered if the data is already pre-loaded prior to createQuery. We need to "sanitize" the state on the server before resolving, so that it can be correctly hydrated back to the client.

Awesome! This is looking great! Thanks for working on this :D

codesandbox-ci · 2023-07-24T15:52:23Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b57acbf:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

codecov-commenter · 2023-07-24T15:53:25Z

Codecov Report

Patch coverage has no change and project coverage change: -13.14 ⚠️

Comparison is base (e0aad73) 97.11% compared to head (d79aef9) 83.97%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff             @@
##             beta    #5775       +/-   ##
===========================================
- Coverage   97.11%   83.97%   -13.14%     
===========================================
  Files          50       11       -39     
  Lines        2391      181     -2210     
  Branches      706       36      -670     
===========================================
- Hits         2322      152     -2170     
+ Misses         67       25       -42     
- Partials        2        4        +2

see 61 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

nx-cloud · 2023-07-24T15:55:32Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b57acbf. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 1 target

nx affected --targets=test:eslint,test:lib,test:types,test:build,test:bundle

Sent with 💌 from NxCloud.

ardeora · 2023-07-25T14:03:34Z

packages/solid-query/src/createBaseQuery.ts

-          resolve(state)
+          const query = observer.getCurrentQuery()
+          resolve(hydrateableObserverResult(query, state))


Awesome! This is looking great! Thanks for working on this :D

ardeora · 2023-07-25T14:06:17Z

packages/solid-query/src/createBaseQuery.ts

-        if (unwrappedResult.isSuccess) {
-          // Use of any here is fine
-          // We cannot include refetch since it is not serializable
-          resolve(unwrappedResult as any)
+        } else {
+          resolve(unwrappedResult)


If we dont call reject here. How do we throw an error to the ErrorBoundary on the server?

ardeora · 2023-07-25T14:10:32Z

packages/solid-query/src/createBaseQuery.ts

-          if (process.env['NODE_ENV'] === 'development') {
-            console.error(unwrappedResult.error)
-          }


We added a log in there because of this discussion https://discord.com/channels/719702312431386674/1072140965910949929

Please let me know if you have a better way to resolve/fix this 😅

apatrida · 2023-12-02T05:36:09Z

@marbemac doesn't this change make the SSR returned object no longer a proxy / store and therefore it is not reactive? See: #6373

Shouldn't it be a Resource or some reactive form other than unwrap to a standard object?

marbemac added 6 commits July 24, 2023 10:45

fix(solid-query): hydrate preloaded data correctly

4a1b93d

Signed-off-by: marbemac <marbemac@gmail.com> # Conflicts: # packages/solid-query/src/createBaseQuery.ts

only include extra props on server

86a60b0

Signed-off-by: marbemac <marbemac@gmail.com> # Conflicts: # packages/solid-query/src/createBaseQuery.ts

fix test watch command

b41b0d8

Signed-off-by: marbemac <marbemac@gmail.com>

update solid-start-streaming example deps

347f6f2

Signed-off-by: marbemac <marbemac@gmail.com> # Conflicts: # examples/solid/solid-start-streaming/package.json # pnpm-lock.yaml

add prefetch example to solid-start-streaming

5aae9ad

Signed-off-by: marbemac <marbemac@gmail.com>

fix: always fall back to resolving

d79aef9

Signed-off-by: marbemac <marbemac@gmail.com>

marbemac commented Jul 24, 2023

View reviewed changes

ardeora approved these changes Jul 25, 2023

View reviewed changes

Merge branch 'beta' into mbm/on-hydrated-fix-beta

b57acbf

ardeora merged commit 7bfecd6 into TanStack:beta Jul 25, 2023

TkDodo mentioned this pull request Feb 17, 2024

Tanstack query results are not always reactive in SolidJS as they sometimes return a non-proxy object #6373

Closed

Uh oh!

fix(solid-query): hydrate preloaded data correctly #5775

fix(solid-query): hydrate preloaded data correctly #5775

Uh oh!

Conversation

marbemac commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codesandbox-ci bot commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jul 24, 2023

Codecov Report

Uh oh!

nx-cloud bot commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apatrida commented Dec 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marbemac commented Jul 24, 2023 •

edited

Loading

vercel bot commented Jul 24, 2023 •

edited

Loading

codesandbox-ci bot commented Jul 24, 2023 •

edited

Loading

nx-cloud bot commented Jul 24, 2023 •

edited

Loading