Open reckter opened 2 years ago
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 :)
@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: https://github.com/xeoneux/next-dark-mode/blob/ca4ac4a6d373425f81158643cade8cc89e58ddd7/src/index.tsx#L106
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.
Ah, that makes sense, thanks for the explanation @lforst! I have updated the lib with the change π
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?)
See https://github.com/getsentry/sentry-javascript/issues/5906
Sentry expects the
getInitialProps
function to return{ pageProps: unknown}
to set something onpageProps
. Currently this HOC, will return{initialProps: {pageProps: unknown }, ...}
. Which will cause Sentry to run into an runtime type error.Our current workaround is:
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.