Feedback on Next.js 13 integration

[anonrig]

I recently tried adding Sentry to my Next.js 13 application - a personal blog of mine available at github.com/anonrig/yagiz.co.

Here's my feedback for the integration process:

1. Running npx @sentry/wizard -i nextjs is not perfect

• It creates next.config.js even though I have a next.config.mjs. Updating existing code might be hard, but it would be nice if the CLI gave me a warning that it created one for me, and I need to update my existing one.

2. Properties file contains user-specific paths

• Script created a .properties file with a unique CLI path for my local machine, but it doesn't make sense to push this to Git, since it differs from person to person, so why do we have it in the first place? (It might make sense in terms of Sentry internals, but from a developer perspective, isn't this against the best practices of Git?)

3. withSentryConfig has a parameter that overrides existing NextConfig type

• It's hard to maintain the config in a single variable, if Sentry is an optional dependency of the product.

4. withSentryConfig receives 2 different configurations

• I didn't understand why do we need SentryNextConfig that extends NextConfig and also a SentryWebpackConfig? Again, it might make sense in terms of the product, but as a developer, isn't simple the best?

5. Sentry wizard does not detect edge

• Until I ran my app locally, I didn't know that sentry.edge.config.ts is required.

6. Why do we have 3 different files? edge, server and client?

• It does not make sense from a user perspective.

[lforst]

Hi, thanks for writing in. Coincidentally I was also reviewing our onboarding for Next.js a bit in the last few days.

• I wasn't aware that the next config could be an .mjs thanks for pointing that out.
• The properties file actually doesn't make sense anymore. You're right. Feel free to delete it. We originally needed it for source maps in local dev but we solve this differently now which doesn't require uploading to Sentry.
• The weird interface of the withSentryConfig function has historic reasons. We needed to be non-breaking in an update. I am planning to update this in the next major.
• You're right the wizard should just create a Sentry edge config. We'll add that.
• The 3 files are not entirely necessary but they're intentional prevent users from footgunning themselves by shipping server code to the client or vice-versa.

Thanks for all the feedback! If you encounter anything else, feel free to drop it here :)

[anonrig]

Thanks for the response @lforst. I've a couple of questions, and more feedbacks:

1. Is there a motivation towards removing any sentry.*.js files and using something universal linked package.json, or having a single file like .eslintrc.js? Or at least having to change the path of those files, and supporting non-root files for the sentry files would be a lot helpful for the file pollution.
2. Can @sentry/nextjs use process.env.SENTRY_DSN || process.env.NEXT_PUBLIC_SENTRY_DSN; environment variables as the default value of Sentry.init so we don't have to write them one by one? Additionally, Sentry can automatically detect if running inside next dev and set the debug: true value.
3. Good to have: Sentry does not detect Vercel environment (preview builds) and automatically set it. Right now it's just development for preview builds. Example: Add sentry anonrig/yagiz.co#3
4. I couldn't find an example for Next.js global-error usage with Sentry. Is it required to setup anything there?

[lforst]

1. No. At least not right now. This is very much a cosmetic change so very down at the bottom of our backlog.
2. While it would be an option I don't think it adds much to DX. You set the DSN once and you're pretty much done with it. You can even hard code it. Exposing it won't really matter since it's visible in the frontend anyways. As for debug: true, I don't think this is a good idea as it can be extremely spammy. We can think about putting into the onboarding snippet to begin - so that during setup it is turned on but people can remove it if they want.
3. The SDK should be picking up the vercel environment via process.env.NEXT_PUBLIC_VERCEL_ENV. Is this not the case for you?
4. What do you mean by "Next.js global-error usage"?

[lforst]

Hi, based on your feedback I completely rewrote the wizard to account for all the cases you ran into. Thanks again for the feedback! :)

[anonrig]

Amazing!