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

fix(nextjs): Remove sentry property from returned next config #5456

Closed
wants to merge 1 commit into from

Conversation

mitchheddles
Copy link

@mitchheddles mitchheddles commented Jul 25, 2022

Next recently added config validation vercel/next.js#38498 so withSentryConfig should remove the additional sentry property to avoid these warnings.

Fixes #5449

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@mitchheddles mitchheddles changed the title Remove sentry property from returned next config fix(nextjs): Remove sentry property from returned next config Jul 25, 2022
@lobsterkatie
Copy link
Member

Hi, @mitchheddles.

Thanks for the contribution!

This is a good idea, but you'll see that some of your tests are failing. This is because of a timing issue, even though I see that you moved constructWebpackConfigFunction to run before the deletion. The problem is that it's not actually this function that needs the sentry data, but rather the function it returns. Since that function doesn't run until later in the build process, by the time it's looking for the data, it's already been deleted.

The fix is to grab it and store it in a closure around the returned function. I tried to make those changes in this PR, but got an auth error when I tried to push. I've therefore created a new PR combining your original idea and the fix: #5473.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning when using hideSourceMaps option in next.config.js
2 participants