Skip to content

fix(nextjs): Fix types in config code #4057

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

Merged
merged 6 commits into from
Oct 12, 2021

Conversation

lobsterkatie
Copy link
Member

This cleans up the types we use in withSentryConfig and friends. Three specific changes:

  • When a user passes their nextjs config to nextjs, they're free to specify as few or as many of the options as they like. Nextjs config objects provided directly by nextjs, on the other hand, will always have certain properties, because they will have been filled in with defaults if not specified by the user. This differentiates between these two cases, through the use of the Partial< ... > utility type when describing config provided by the user.

  • In order to play more nicely with @sentry/webpack-plugin and its types, this uses the official WebpackPluginInstance type from webpack itself. This requires including webpack as a dependency, in this case a peer dependency, since we're only using it for its types and therefore don't actually care if it's installed or not. (Given nextjs's reliance on it, you might be asking yourself how a nextjs user would ever not have it already installed, and the answer is that nextjs ships with a precompiled version and therefore doesn't include it as anything other than a dev dependency.)

  • Given that we still support webpack 4, this ramps our types dev dependency down to the lowest version which includes the compatibility type WebpackPluginInstance.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 22.34 KB (0%)
@sentry/browser - Webpack 23.31 KB (0%)
@sentry/react - Webpack 23.34 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 29.79 KB (+0.01% 🔺)

@lobsterkatie lobsterkatie force-pushed the kmclb-nextjs-fix-config-types-oct-2021 branch from 90fc36b to 1b46c28 Compare October 12, 2021 05:30
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and all tests, including integration tests for next.js are passing, so 👍

Copy link
Contributor

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@lobsterkatie lobsterkatie merged commit 0de61f8 into master Oct 12, 2021
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-fix-config-types-oct-2021 branch October 12, 2021 19:45
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.

3 participants