feat(nextjs): Deprecate Webpack top-level options - #18343
Conversation
RulaKhaled
left a comment
There was a problem hiding this comment.
Generally looks good to me, but i'll recommend waiting for @charly's review!
…level ones for deprecation
…dd deprecation warnings for top-level options
beb7c72 to
9ef882c
Compare
chargome
left a comment
There was a problem hiding this comment.
Looks good already, let's make sure to have a docs pr in place that we can deploy immediately when this is released otherwise users will have a conflict between docs|sdk
chargome
left a comment
There was a problem hiding this comment.
Just blocking this for widenClientFileUpload so this does not get merged accidentally
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
|
@chargome I got a companion docs PR ready as well for this so this should be ready to go if you think so. |
There was a problem hiding this comment.
Pull request overview
This PR deprecates webpack-specific configuration options at the top level of Sentry build options and introduces a new webpack namespace to better organize these settings. The change is non-breaking due to a comprehensive backward compatibility layer that migrates deprecated options while emitting deprecation warnings.
- Adds a new
SentryBuildWebpackOptionstype containing all webpack-specific configuration - Implements automatic migration from deprecated top-level options to the new
webpack.*path - Updates all internal references to use the new namespaced configuration
- Maintains full backward compatibility with deprecation warnings for future removal
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nextjs/src/config/types.ts | Defines new SentryBuildWebpackOptions type and adds @deprecated annotations to top-level webpack options; introduces new webpack property on SentryBuildOptions |
| packages/nextjs/src/config/withSentryConfig.ts | Implements migrateDeprecatedWebpackOptions() function to handle backward compatibility and emit deprecation warnings; updates webpack config construction to use new namespaced options |
| packages/nextjs/src/config/webpack.ts | Updates all references from top-level options to webpack.* namespaced options |
| packages/nextjs/src/config/getBuildPluginOptions.ts | Updates plugin option extraction to read from webpack.* namespace |
| packages/nextjs/test/config/withSentryConfig.test.ts | Adds comprehensive test coverage for migration logic, precedence rules, and deprecation warnings |
| packages/nextjs/test/config/getBuildPluginOptions.test.ts | Updates existing tests to use new webpack.* namespaced options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| treeshake?: { | ||
| /** | ||
| * Tree shakes Sentry SDK logger statements from the bundle. Note that this doesn't affect Sentry Logs. |
There was a problem hiding this comment.
The documentation should be more explicit about what setting this option to true does. Consider updating it to:
/**
* When set to `true`, tree shakes Sentry SDK logger statements from the bundle, reducing bundle size.
* Note that this doesn't affect Sentry Logs.
* Defaults to `false`.
*/
debugLogging?: boolean;This makes it clear that setting it to true enables the tree-shaking (removal) behavior.
| * Tree shakes Sentry SDK logger statements from the bundle. Note that this doesn't affect Sentry Logs. | |
| * When set to `true`, tree shakes Sentry SDK logger statements from the bundle, reducing bundle size. | |
| * Note that this doesn't affect Sentry Logs. | |
| * Defaults to `false`. |
chargome
left a comment
There was a problem hiding this comment.
Looks good, just double checking: did you test this out locally and confirmed the webpack options get set correctly? We can't break existing setups with this one
|
Yep, I tested every option and made sure they are set correctly during build, will just fix a quick comment and merge. |
…5691) We are moving the only-webpack related configurations under `webpack` namespace to better fit our goal of communicating the SDK readiness for Turbopack by default. This involved moving several options to be nested under a `webpack` path in the build configuration. This is meant to be released once this getsentry/sentry-javascript#18343 goes live, so I will keep this open until then. Will keep it in draft as well. The SDK PR adds warnings for those deprecated options and warns users about using them if detected so I don't think we need to do that here once it goes live. --------- Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Removes the deprecated top-level `withSentryConfig` options marked with `TODO(v11)`. All have replacements documented as an old→new table in `docs/migration/v11-end-state.md`. Also removes top-level `excludeServerRoutes`, which was deprecated in the same PR as the others (#18343) but never got a `TODO(v11)` marker. closes #22245 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This PR deprecates the webpack-only configurations via the
@deprecatedJSDoc annotation and introduces a newwebpackconfig namespace for them.Under the hood the logic was changed to read from the new values, with a compatibility layer that sets them from the deprecated top-level options while warning for each option if used.
This should set us up for a v11/v12 deletion of those options.
I might have missed a few options that only affect webpack, so I appreciate a good look at this. At any case this isn't breaking so even if missed a few, users won't experience disruptions.