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.

Sign up for GitHub

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

Jump to bottom

bugfix: Nextjs SSR issues (fixes #822) #823

Merged
merged 5 commits into from
Apr 17, 2023
Merged

bugfix: Nextjs SSR issues (fixes #822) #823

merged 5 commits into from
Apr 17, 2023

Conversation

jmyrland
Copy link
Collaborator

@jmyrland jmyrland commented Mar 30, 2023 •
edited
Loading

It seems that this issue is spawned by the useSyncExternalStoreWithSelector is initialized with getServerSnapshot set to an object, when a function is expected - because the getServerState (from createStore()) returns the initial state directly.

I've refactored this, to set the getServerSnapshot equal to the getState method from the store.
As I no longer see the need for the getServerState (from createStore()), I've removed it.

  • Bumps nextjs example to latest version (does not reproduce the issue)
  • Adds a nextjs-ssr example (reproduces the issue)
  • Extends server render tests

related to #822

@vercel
Copy link

vercel bot commented Mar 30, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
easy-peasy ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 2, 2023 10:10am

@jmyrland jmyrland marked this pull request as ready for review April 2, 2023 10:17
no-stack-dub-sack
no-stack-dub-sack approved these changes Apr 12, 2023
View reviewed changes
Copy link
Collaborator

@no-stack-dub-sack no-stack-dub-sack left a comment

Choose a reason for hiding this comment

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

I didn't dig deep into reviewing the examples, but the core code change makes sense and looks good to me.

@jmyrland jmyrland mentioned this pull request Apr 12, 2023
Closed
@jmyrland jmyrland changed the title bug: Nextjs issues (#822) bug: Nextjs SSR issues (#822) Apr 12, 2023
@jmyrland jmyrland changed the title bug: Nextjs SSR issues (#822) bugfix: Nextjs SSR issues (#822) Apr 17, 2023
@jmyrland jmyrland changed the title bugfix: Nextjs SSR issues (#822) bugfix: Nextjs SSR issues (fixes #822) Apr 17, 2023
@jmyrland jmyrland merged commit 7e13adf into master Apr 17, 2023
@jmyrland jmyrland deleted the bug/822-nextjs-issues branch April 17, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@no-stack-dub-sack no-stack-dub-sack no-stack-dub-sack approved these changes

@ctrlplusb ctrlplusb Awaiting requested review from ctrlplusb

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jmyrland @no-stack-dub-sack