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

fix getInitialPageProps return #378

Closed

Conversation

alikazemkhanloo
Copy link

I think this part is forgotten.

I think this part is forgotten.
@kirill-konshin
Copy link
Owner

What issue are you trying to solve? initlalProps and initialState are supposed to be unwrapped during rendering. Can you provide a codesandbox for me to see the scenario when current implementation fails?

@alikazemkhanloo
Copy link
Author

Hi,
Current implementation doesn't fail. But normally when I return an object in getInitialProps, it would be available as props, not as a prop inside props.

When i return

{
  name:'a'
}

In getInitialProps, I expect to get name directly in props. when I return the same object in wrapper.getInitialPageProps, I get props.initialProps.name. I think this wrapper should behave like the original function.

@kirill-konshin
Copy link
Owner

This is exactly how it works. You will get your props unwrapped. The internal objects are needed for transport, you should never see them in your own components. If you do - that’s a bug, and it should be fixed. Can you create a sandbox to illustrate it if you see the internals.

@alikazemkhanloo
Copy link
Author

sandbox

@kirill-konshin
Copy link
Owner

image

@alikazemkhanloo
Copy link
Author

fixed

@kirill-konshin
Copy link
Owner

I see now, thank you. Let me check why they are leaking.

kirill-konshin added a commit that referenced this pull request Jun 11, 2021
@kirill-konshin
Copy link
Owner

This has to be fixed slightly differently, I'm closing this PR, I've referenced the PR in the correct fix fc056a9. Thank you for contributing.

@ypresto
Copy link

ypresto commented Jun 12, 2021

@kirill-konshin

At least in my environment I got props inside props still after your fix.

cd packages/demo && npm start then localhost:3000/pageProps shows:

image

@ypresto
Copy link

ypresto commented Jun 12, 2021

It was because just I forget to run npm build on parent dir. Sorry >_<

@ypresto
Copy link

ypresto commented Jun 12, 2021

@kirill-konshin

There is another but similar issue that HYDRATE dispatched twice with eventually empty state, when wait for saga.
Reproduced in demo-saga modified to use getInitialPageProps().
It is because getInitialPageProps() returns { initialState } so

if (initialStateFromGSPorGSSR)
this line executed after first HYDRATE.

sandbox:
https://codesandbox.io/s/next-redux-wrapper-getinitialpageprops-saga-bug-zfkoi

Just we should wait for saga in getInitialPageProps().

@alikazemkhanloo alikazemkhanloo deleted the patch-1 branch July 12, 2021 07:34
@alikazemkhanloo alikazemkhanloo restored the patch-1 branch July 12, 2021 07:34
@alikazemkhanloo alikazemkhanloo deleted the patch-1 branch July 12, 2021 07:35
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.

3 participants