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 getServerSnapshot, fix loop on SSR Subscribe #306

Merged
merged 2 commits into from
Aug 4, 2023
Merged

Add getServerSnapshot, fix loop on SSR Subscribe #306

merged 2 commits into from
Aug 4, 2023

Conversation

voliva
Copy link
Collaborator

@voliva voliva commented Aug 4, 2023

Fixes #305

Something important to keep in mind is that in SSR any children within a Subscribe will not render, because given that components never mount in SSR (including <Subscribe>), it will never get into the stage of rendering the children.

And StateObservable with top-level subscriptions will have their state shared between clients.

@voliva voliva requested a review from josepot August 4, 2023 09:06
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #306 (54bfb1a) into main (963ff94) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #306   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        25    +1     
  Lines         1232      1279   +47     
  Branches       141       148    +7     
=========================================
+ Hits          1232      1279   +47     
Files Changed Coverage Δ
packages/core/src/Subscribe.tsx 100.00% <100.00%> (ø)
...ore/src/test-helpers/pipeableStreamToObservable.ts 100.00% <100.00%> (ø)
packages/core/src/useStateObservable.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@voliva voliva marked this pull request as ready for review August 4, 2023 10:26
Comment on lines -49 to +53
args: [, gv] as any,
args: [, gv, gv] as any,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can use gv for serverSnapshot because everything still matches:

    const gv: <T>() => Exclude<T, typeof SUSPENSE> = () => {
      const src = callbackRef.current!.source$ as DefaultedStateObservable<O>
      if (!src.getRefCount() && !src.getDefaultValue) {
        // `subscription` will always be null, because it's impossible for a component to run this hook if it's inside a <Subscribe>
        // Then this will happen only if a component without a <Subscribe> is using a StateObservable that doesn't have a subscription => Missing Subscribe
        if (!subscription) throw new Error("Missing Subscribe!")
        subscription(src)
      }
      // The rest should also work:
      // Observables with top-level subscriptions will return their value, or trigger Suspense
      // Observables without subscriptions and defaultValue will return their defaultValue
      return getValue(src)
    }

Copy link
Member

@josepot josepot left a comment

Choose a reason for hiding this comment

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

Nice! Thanks a lot @voliva !

@voliva voliva merged commit d0d089a into main Aug 4, 2023
3 checks passed
@voliva voliva deleted the ssr branch August 4, 2023 11:50
@CMCDragonkai
Copy link

Does this means one should not use this in server side components?

@voliva
Copy link
Collaborator Author

voliva commented Aug 5, 2023

Does this means one should not use this in server side components?

At the moment I don't really have an answer for this, I haven't played that much with SSR or server-side components.

It's just important to keep in mind that in server-side:

  1. Anything inside <Subscribe> will not render. <Subscribe> is used to subscribe to a source$ before the children are rendered. If <Subscribe> can't subscribe to the source because it never gets mounted, then it will never render its children.
  2. StateObservables with defaultValue will return their default value (just as normal).
  3. Any StateObservables with a subscription somewhere else (e.g. a top-level subscription) will have their state shared between every render. This could be useful, or it could be annoying to get around.

My best advice (again, without having played that much with SSR/NextJS) is to have every StateObservable with a defaultValue on those components that will run on the Server. It gives the most predictable and simple results: They will return the defaultValue while on the server, and execute the subscription when on the client.

Maybe on a future we can leverage something like reactjs/rfcs#229 so that server components also subscribe to the observables, but there are some serious questions regarding cancellation and state sharing.

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.

NextJS 13 getServerSnapshot issue
3 participants