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

NextJS 11 withSentryConfig returns a function when it should return an object #3722

Closed
4 of 9 tasks
tm1000 opened this issue Jun 21, 2021 · 2 comments · Fixed by #3760
Closed
4 of 9 tasks

NextJS 11 withSentryConfig returns a function when it should return an object #3722

tm1000 opened this issue Jun 21, 2021 · 2 comments · Fixed by #3760
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@tm1000
Copy link

tm1000 commented Jun 21, 2021

Package + Version

  • @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other: @sentry/nextjs

Version:

6.7.2

Description

Utilizing https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/ withSentryConfig is returned directly into module.exports. However it's a function at this level and needs to be executed when sent to module.exports to actually send the settings to NextJS. Otherwise it's just a function that nextjs never exports. This actually ends up breaking any settings anyone has added in Next.config.js because once withSentryConfig does this then nextjs sees theres no webpack key on the object sent to module.exports

@tm1000
Copy link
Author

tm1000 commented Jun 21, 2021

This was due to this PR #3698

Which in reality. The issue was how my next.config.js was setup. What I was doing was running withSentryConfig THEN adding my additional settings to settings. But what I should have been doing was adding my additional settings then sending them into withSentryConfig.

Before:

const settings = withSentryConfig()
settings.eslint = {
	ignoreDuringBuilds: true
};

module.exports = settings;

After:

const settings = {};
settings.eslint = {
	ignoreDuringBuilds: true
};

module.exports = withSentryConfig(settings);

This was a pretty big breaking change for us... probably because we were doing things wrong initially but it's bound to bite additional people

cc @lobsterkatie

@lobsterkatie
Copy link
Member

lobsterkatie commented Jun 28, 2021

@tm1000 - Thanks for pointing this out.

In that PR I changed withSentryConfig to output a function, because otherwise nextjs won't pass phase and defaultConfig to us in order that we be able to forward them on to any existing config function. That said, that's only necessary if the existing confg that's passed to us is, in fact, a function. I can add a check, and revert to the old return-an-object behavior if we're not given a function.

lobsterkatie added a commit that referenced this issue Jun 29, 2021
…ype (#3760)

When fixing https://github.com/getsentry/sentry-docs/issues/3723, the return type of `withSentryConfig` was changed from an object to a function which returns an object, since otherwise there's no way to get the `phase` and `defaultConfig` arguments needed when the user passes their existing nextjs config as a function.

If users follow our instructions in https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/, and export `withSentryConfig(...)` with no further modifications, this is an invisible change, since they're never dealing with its return value. But for any who do further process said value, it turns out to have been a potentially breaking change.

This reverts to the old behavior (of returning an object) in cases where the existing config is an object, and only returns a function when necessary, which is to say, when the existing config is itself a function.

Fixes #3722.
@lobsterkatie lobsterkatie added Package: nextjs Issues related to the Sentry Nextjs SDK Status: Fixed Type: Bug and removed Status: In Progress labels Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants