-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature/rtkq ssr #1270
Feature/rtkq ssr #1270
Conversation
✔️ Deploy Preview for redux-starter-kit-docs ready! 🔨 Explore the source changes: 804cea0 🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/60e71e0f4c83300007cc0558 😎 Browse the preview: https://deploy-preview-1270--redux-starter-kit-docs.netlify.app |
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 804cea0:
|
Going to page @Ephem to see if he's got any thoughts here. |
) | ||
) { | ||
// ssr mode is on and there is no promise yet | ||
const promise = dispatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just leaving this comment to point out to anyone not used to SSR that this dispatch is the necessary "hack" to make this whole approach work. Since useEffect
doesn't run on the server, when in ssr mode this PR would make the dispatch fire directly in render instead, breaking the no observable side effects in render rule. The new SSR renderer in React 18 will be pretty likely to break/break with this approach (and indeed any approach that fetches data from within the tree).
Because of how SSR works pre React 18, this is pretty much the only way to do it if you want to support fetching data from within the component tree, instead of only ahead of rendering. So while not an uncommon approach, I wanted to clearly mark out the "hacky part" to others. 😄
(I'll leave some more thoughts and comments on the linked issue instead of here on the PR since I think they fit better there. Update: Comment added here)
I believe this is all superseded by the work in #1277 . Thanks! |
This is the Proof of Concept for the
getDataFromTree
implementation mentioned in this discussion #1184. Please let me know what you think.