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(example)Next JS _document error #1490

Merged
merged 1 commit into from Jun 30, 2022
Merged

fix(example)Next JS _document error #1490

merged 1 commit into from Jun 30, 2022

Conversation

TIMMLOPK
Copy link
Contributor

@TIMMLOPK TIMMLOPK commented Jun 29, 2022

Overview
If using nextjs =>12.1.6 , may get error: "MyDocument.getInitialProps()" should resolve to an object with a "html" prop set with a valid html string

Documentation

  • Updated Typescript types and/or component PropTypes
  • Added / modified component docs
  • Added / modified Storybook stories

@netlify
Copy link

netlify bot commented Jun 29, 2022

Deploy Preview for evergreen-storybook ready!

Name Link
🔨 Latest commit 911f495
🔍 Latest deploy log https://app.netlify.com/sites/evergreen-storybook/deploys/62bbdbff1ec75800084fe8c8
😎 Deploy Preview https://deploy-preview-1490--evergreen-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cogwizzle
Copy link
Contributor

cogwizzle commented Jun 29, 2022

Would this break any existing functionality? I can see that the current version supports the logic that you recommended but I am just curious if it would break existing codebases that haven't updated to 12.x yet. I can't find older documentation, but here is some documentation from NEXT supporting @TIMMLOPK's change. https://nextjs.org/docs/advanced-features/custom-document#customizing-renderpage

@brandongregoryscott
Copy link
Contributor

@TIMMLOPK Did you mean v12.2+? I don't think NextJS has a v12.6 out yet: https://www.npmjs.com/package/next

In either case, I'm not sure I'm able to reproduce this. Can you provide a CodeSandbox for reproduction?

@TIMMLOPK
Copy link
Contributor Author

@TIMMLOPK Did you mean v12.2+? I don't think NextJS has a v12.6 out yet: https://www.npmjs.com/package/next

In either case, I'm not sure I'm able to reproduce this. Can you provide a CodeSandbox for reproduction?

It is 12.1.6,I correct it.I would try to make reproduction

@TIMMLOPK
Copy link
Contributor Author

@brandongregoryscott seem can't make it in codesandbox because it crash.I made a repo for it :https://github.com/TIMMLOPK/error-reproduction

@TIMMLOPK
Copy link
Contributor Author

Would this break any existing functionality? I can see that the current version supports the logic that you recommended but I am just curious if it would break existing codebases that haven't updated to 12.x yet. I can't find older documentation, but here is some documentation from NEXT supporting @TIMMLOPK's change. https://nextjs.org/docs/advanced-features/custom-document#customizing-renderpage

No , I tried to use 11.x ,it doesn't matter

@brandongregoryscott
Copy link
Contributor

Ah, ok. I see the issue in my console using that version (and again when I updated to 12.2.0 - not sure why I wasn't seeing it before, perhaps it wasn't actually running the latest version)

This change seems to work in the version we have specified in the package.json too (12.1.0), so I think this is fine to update. Thanks!

@brandongregoryscott brandongregoryscott merged commit 79a43c3 into segmentio:master Jun 30, 2022
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.

None yet

3 participants