Together with @sentry/nextjs this will break production builds

[reckter]

See getsentry/sentry-javascript#5906

Sentry expects the getInitialProps function to return { pageProps: unknown} to set something on pageProps.
Currently this HOC, will return {initialProps: {pageProps: unknown }, ...}.
Which will cause Sentry to run into an runtime type error.

Our current workaround is:

// _app.ts

function MyApp({ Component, pageProps }) {
  return <Component {...pageProps} />
}

MyApp.getInitialProps = async (appContext: AppContext) =>
	App.getInitialProps(appContext)

const WithDarkApp = withDarkMode(MyApp)
const oldInitialProps = WithDarkApp.getInitialProps

// This fixes, that sentry expects there to be no extra `initialProps` field, and instead directly expects the `pageProps` field
WithDarkApp.getInitialProps = async (appContext: AppContext) => {
	const initialProps = await oldInitialProps(appContext)
	return {
		...initialProps?.initialProps,
		...initialProps,
	}
}

export default WithDarkApp

I think ideally this should be fixed in the library directly, but it could theoretically break some apps, if they rely on this current behavior.

If desired, I might be able to come up with a fix myself (if my time permits 😓), but wanted to gauge feedback first.

[xeoneux]

Hi, @reckter thanks for reporting this!
I have pushed a beta version 4.0.0-beta.0 with the changes that you requested.
Try that out and let me know if it solves your issue with Sentry :)

[lforst]

@xeoneux Hi, Sentry SDK maintainer here. Thank you for taking a look at this!

I am not entirely sure whether your fix will work since your wrapper still doesn't return { pageProps: unknown } when the user hasn't defined a getInitialProps in his _app.

The problematic line is:

next-dark-mode/src/index.tsx

Line 106 in ca4ac4a

const appProps = App.getInitialProps ? await App.getInitialProps(appContext) : {}

The Next.js docs state that if you have a getInitialProps in your _app, you must call App.getInitialProps(appContext) and merge the result.
In the case of your wrapper, you are setting getInitialProps for the user if it is not there, so you must also always call import("next/app").getInitialProps(appContext) for them.

I propose the following change:

- const appProps = App.getInitialProps ? await App.getInitialProps(appContext) : {}
+ import NextApp from "next/app"
+ const appProps = App.getInitialProps ? await App.getInitialProps(appContext) : await NextApp.getInitialProps(appContext)

We should probably also be a bit more defensive in what data types we assert in the SDK but I still recommend you go through with this change just to be in line with the Next.js docs.

[xeoneux]

Ah, that makes sense, thanks for the explanation @lforst! I have updated the lib with the change 👍

[reckter]

Tested with 4.0.0-beta.1, and that seems to fix it! 👍
(Sidenode: I had to revert our repository to the original dependencies versions, maybe sentry actually put in a workaround at their end?)