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

Update Advanced Error Page section in custom-error-page.md #32873

Closed
wants to merge 1 commit into from
Closed

Update Advanced Error Page section in custom-error-page.md #32873

wants to merge 1 commit into from

Conversation

glenngijsberts
Copy link
Contributor

Description

Update More Advanced Error Page Customizing section in custom-error-page.md, by using getServerSideProps instead of getInitialProps.

Tested locally in my own project and seems to be working as expected. However, on the same docs page there is an example that is using getServerSideProps for the page where the Error component is being used so i'm wondering: is there a reason why the example of the custom Error page isn't using getServerSideProps? I was thinking that it maybe had the same constraint as the custom Document & App pages, where Next.js Data Fetching methods like getServerSideProps are not supported.

If there are no constraints, this update seems good to me, because it aligns with the latest practices of Next.js. Otherwise maybe a good idea to add the same line as https://nextjs.org/docs/advanced-features/custom-app#caveats where it says that those data fetching methods are not supported.

Documentation / Examples

  • Make sure the linting passes by running yarn lint

Update More Advanced Error Page Customizing section in custom-error-page.md, by using `getServerSideProps` instead of `getInitialProps`.
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Hi, getServerSideProps isn't fully compatible as a replacement for getInitialProps with _error here as we don't provide the err in the context passed to getServerSideProps so I don't think we should update to recommend this in the documentation here currently.

@glenngijsberts
Copy link
Contributor Author

@ijjk right, was already thinking something like that is the case. WDYT about adding a "caveats" section with this info included? Like there is for _document and _app?

@ijjk
Copy link
Member

ijjk commented Jan 1, 2022

Yeah might be good to add a caveat mentioning that getServerSideProps is not officially supported in _error

@balazsorban44
Copy link
Member

@glenngijsberts are you interested in updating the Caveats section?

@glenngijsberts
Copy link
Contributor Author

@balazsorban44 @ijjk yes! I'll make sure to update that, and i'll close this :)

@glenngijsberts glenngijsberts deleted the patch-1 branch January 10, 2022 18:33
kodiakhq bot pushed a commit that referenced this pull request Jan 10, 2022
## Desscription

This is a follow up of #32873. As discussed in #32873, currently it's not recommended to use getServerSideProps in the `Error` component, so this PR will add that caveat to the documentation page, like `Document` and `App` also have.

## Documentation / Examples

- [x] Make sure the linting passes by running `yarn lint`
@vercel vercel locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants