Hydration fix

[kirill-konshin]

Refactored wrapper.useWrappedStore() to just return store with no hydration. All hydration was moved into wrapper.useHydration(props).

Props logic was simplified.

Memo hydration is now called only once per page load, to accommodate server side rendered HTML. All subsequent hydrations are dispatched by layout effect. This means all selectors must take into account the fact that data may be unavailable.

This will be released as 9.x major release.

@voinik pls have a look.

Fixes #522
Fixes #520

Honestly, I find it really weird that old component is not unmounted when new is mounting. Sounds like a bug.

[kirill-konshin]

Btw, I also added a HOC for legacy pages wrapper.withHydration(Page), which lead me to idea, what if this HOC will take optional loading component and prevent showing of the page, that is not ready yet:

function Page () {
  const foo = useSelector(...);
  return <div>{foo}</div>;
}

function Loading () { return <div>Loading</div>; }

export default wrapper.withHydration(Page, { Loading });

In this case hook-based wrapper.useHydration is no longer needed.

This smells like old school HOC approach, but abstracts implementation nuances from users.

Alternative API may look like this:

function Page () {
  const { loadingSelector, loading } = wrapper.useHydration();
  const foo = useSelector(loadingSelector(someRealSelector, 'someDefaultValueMaybe?'));
  if (loading) return <div>Loading</div>;
  return <div>{foo}</div>;
}

@daniil4 @voinik @Pikachews @bjoluc thoughts?

[kirill-konshin]

Implemented the above. Updated readme can be found here: https://github.com/kirill-konshin/next-redux-wrapper/tree/hook-api

I significantly simplified everything. Seems to be the right direction.

[bjoluc]

Looks like the right step to me and I love the new simplicity – great job! I'm only a little worried about server-side hydration behavior: Will the server-rendered markup contain anything that's introduced with the HYDRATE action (such as state from cookies in next-redux-cookie-wrapper)?

[kirill-konshin]

Few more considerations regarding useHydration vs withHydration:

1. Hook is more modern, but requires fixing of selectors (state => state?.foo?.bar), which is simple, but consumers will have to do it as part of migration
2. HOC is older style, and most of libraries these days are abandoning it, but HOC is a bulletproof solution to prevent render of incomplete selectors
3. BUT in general selectors should always be safe because:
   a. there's no guarantee when portions of state will arrive, and
   b. same selectors may be used on the particular page which dispatches hydration and elsewhere in the app, and HOC will not protect against this

So I am considering to just return loading status without loadingSelector, as this would enforce better coding practices.

Honestly, I have a question to authors of useSelector, because it can't handle useSelector(null), unlike, say, SWR, where useSWR(condition ? '/api/foo' : null) is a perfectly legit pattern.

[bjoluc]

So I am considering to just return loading status without loadingSelector, as this would enforce better coding practices.

Yup, fully on board with that. Folks should either use optional chaining in a selector, or let the store ensure the selected path is available at any time. Let's keep things simple on our end; the Readme is bloated enough :D

[kirill-konshin]

So I am considering to just return loading status without loadingSelector, as this would enforce better coding practices.

Yup, fully on board with that. Folks should either use optional chaining in a selector, or let the store ensure the selected path is available at any time. Let's keep things simple on our end; the Readme is bloated enough :D

Readme have been shortened a bit, and I will cut it down even more ;) it's a direct consequence of tricky Next.js props management. And it will be even worse if I will find a way to make wrapper to work with Server Components and new app folder.

[kirill-konshin]

I just had a breakthrough moment. I have ditched HYDRATE action, and started to just replay all the actions that happened on server. No more issues with tricky hydration, no more server/client state separation problems. I have deleted a few sections of readme, it's now much simpler.

[bjoluc]

I just had a breakthrough moment. I have ditched HYDRATE action, and started to just replay all the actions that happened on server. No more issues with tricky hydration, no more server/client state separation problems. I have deleted a few sections of readme, it's now much simpler.

Congratulations! 🎉 The middleware approach looks really good! I wonder how we didn't consider replaying actions before 😅 Of course, this means actions and reducers must be designed s.t. dispatching the same action multiple times does not alter state (i.e., no toggle actions and friends). But that seems like a fair tradeoff for not needing to maintain HYDRATE reducers! In this regard, I wonder if we should take some

ignoreActions: (action: AnyAction) => boolean

config option in case some actions are not safe/meant to be replayed?

[bjoluc]

Looks very good. I'm in favor of renaming the page props from initialStateABC to reduxActionsABC since it's no initial state but actions. And I'd like to have redux in the name to somehow make clear where the props come from / where they belong to. WDYT?

[bjoluc]

In this regard, I wonder if we should take some

ignoreActions: (action: AnyAction) => boolean

config option in case some actions are not safe/meant to be replayed?

Don't mind my words, I hadn't reviewed index.tsx yet, and hadn't discovered actionFilter 🤦

[kirill-konshin]

Looks very good. I'm in favor of renaming the page props from initialStateABC to reduxActionsABC since it's no initial state but actions.

Done.

I wonder how we didn't consider replaying actions before 😅

Better late than never... HYDRATE was also fine. I started to brainstorm how to implement wrapper for new app directory, and got an idea to push updates in a granular way, because there any server component can push bits of new state. But turned out Redux can't be used on server, at least for now. Nevertheless, the idea was quite useful.

[DamienDeloubes]

Any updates on this 😎. Just experienced this issue also.

[bjoluc]

Dear @DamienDeloubes,

as you can see from the activities listed in this PR, @kirill-konshin is actively working on this. The fix became a major rewrite and releasing it with confidence is a time-consuming job, given that @kirill-konshin is mostly left alone working on this library, despite its popularity. So please be patient and consider reading this post on issue bumping.

Thanks!

[kirill-konshin]

Latest update — I have finally fixed all tests, they are all passing and it all seems to be working fine.

I plan to release it to master, to let people test it in real apps.

You guys are welcome to provide more testing, if you can.

[bjoluc]

Great news! I'll likely find some time to plug this into an existing app tomorrow. If you'd like me to, I can also work on improving test coverage then. Re master, let me refer to #194 (comment) – if this becomes master (btw, time to rename to main?), the readme should at least have a notice with a link to the v8 readme at a prominent position...

[kirill-konshin]

Great news! I'll likely find some time to plug this into an existing app tomorrow. If you'd like me to, I can also work on improving test coverage then. Re master, let me refer to #194 (comment) – if this becomes master (btw, time to rename to main?), the readme should at least have a notice with a link to the v8 readme at a prominent position...

It would be great if you can test it out.

I usually just switch default branch, so that when you open the repo, v7 or v8 is shown.

I have just fixed the coverage too, btw.

[kirill-konshin]

V8 is now default branch, this PR has been committed to master.

[sebastiangrebe]

@kirill-konshin can we get a prerelease for the v9? I am struggling testing it out on my local with master

[kirill-konshin]

@kirill-konshin can we get a prerelease for the v9? I am struggling testing it out on my local with master

https://github.com/kirill-konshin/next-redux-wrapper/releases/tag/9.0.0-rc.1
https://www.npmjs.com/package/next-redux-wrapper/v/9.0.0-rc.1

[fendermany]

Hi Kirill,
Thank you for your efforts. I installed the latest version 9, but I don't understand how to use it with getStaticPaths

Code below gives me error useMemo of undefined

[kirill-konshin]

You can't dispatch from getStaticPaths. It is used only to create a number of paths, which then are processed by getStaticProps, one by one.

[magicDvlp]

You can't dispatch from getStaticPaths. It is used only to create a number of paths, which then are processed by getStaticProps, one by one.

It seems the question is about using rtk query and next-redux-wrapper. This is the recommended way to get data inside getStaticProps. Also without the HYDRATE action, the recommended way to use rtk query with the next-redux-wrapper is no longer possible.
Can you give a couple of tips on how to use rtk qveri with next-redux-wrapper now.

[kirill-konshin]

Why not possible? All actions are recorded and re-dispatched on client. https://github.com/kirill-konshin/next-redux-wrapper/tree/master/packages/demo-redux-toolkit use the example. Just make sure to dispatch all actions from getStaticProps, and wrap it properly. getStaticPaths can remain unwrapped, you only need it to get the paths. This is how Next.js is architected.

[magicDvlp]

But if I use rtk query to get the number of paths in getStaticPaths, I need to dispatch an action to call the endpoint. As shown in the screenshot.
And do I understand correctly that this way of using rtl query is no longer supported?

[fendermany]

I tried getting all patches in getStaticPatch with fetch, but finally, I got 404 when accessing a mapped page. And of course, you can't use getStaticProps without getStaticPaths on [param] pages. It looks like we need to check that the paths are created before doing anything else. Any ideas?

[Distortedlogic]

any update on when there will be a new release with this fix?

[its-monotype]

Any updates on when it will be released?

[doskovicmilos]

The solution with wrapper.useHydration(props); will not perform rendering on SSR in Layout if getLayout is used for Per-Page Layouts.