-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): added webpack treeshaking flags as config #18359
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
feat(nextjs): added webpack treeshaking flags as config #18359
Conversation
size-limit report 📦
|
bf5a673 to
66aa19c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the Sentry Next.js SDK's tree-shaking capabilities by renaming debugLogs to debugLogging and introducing four new configuration options for fine-grained control over SDK bundle inclusion.
Key Changes:
- Renamed configuration option from
debugLogstodebugLoggingfor consistency - Added
tracing,excludeReplayIframe,excludeReplayShadowDOM, andexcludeReplayCompressionWorkertree-shaking options - Refactored webpack configuration to use a dedicated
setupTreeshakingFromConfigfunction for improved maintainability
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| packages/nextjs/src/config/types.ts | Added type definitions and JSDoc documentation for four new tree-shaking options |
| packages/nextjs/src/config/webpack.ts | Extracted tree-shaking setup into a dedicated function that handles all five tree-shaking flags via webpack DefinePlugin |
| packages/nextjs/test/config/fixtures.ts | Updated DefinePlugin mock to accept and store definitions for test verification |
| packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts | Added comprehensive test suite covering all tree-shaking options across different build contexts (server, client, edge) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
03e070a to
e66a724
Compare
packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts
Outdated
Show resolved
Hide resolved
packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts
Outdated
Show resolved
Hide resolved
e66a724 to
c8832af
Compare
packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts
Outdated
Show resolved
Hide resolved
de51fb2 to
daa72ef
Compare
daa72ef to
261ad56
Compare
chargome
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice DX improvement
|
@logaretm make sure to add a section in the changelog for this 🙏 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
261ad56 to
825cf35
Compare
| expect(treeshakePlugin.definitions).not.toHaveProperty('__SENTRY_TRACING__'); | ||
| expect(treeshakePlugin.definitions).not.toHaveProperty('__RRWEB_EXCLUDE_SHADOW_DOM__'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing integration or E2E tests for feature (Bugbot Rules)
Per the review rules specified in this repository, feat PRs should include at least one integration or E2E test. This PR adds new webpack treeshaking configuration flags but only includes unit tests. Since these flags affect the actual webpack build behavior and bundle output, an integration or E2E test verifying that the treeshaking actually works as expected in a real Next.js build would provide stronger confidence in the feature. Flagged because review rules file requests this check.
# What A PR to go in tandem with getsentry/sentry-javascript#18359 once it gets merged and released. It documents the new tree-shaking options now abstracted away into `webpack.treeshake` namespace. It has a few differences to the existing tree-shaking flags under the hood: - All options are optimized to be set to `true` to take effect, unlike the flags where some had to be set to `true` and some had to be set to `false`. - Now exposed as an SDK `webpack.treeshake` build options rather than needing the user to be aware of the webpack define plugin API. Marking it as a draft to avoid merging it before the PR goes live. --------- Co-authored-by: Charly Gomez <charly.gomez@sentry.io> Co-authored-by: Sarah Mischinger <sarah@codingwriter.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR updates the Sentry Next.js integration to improve tree-shaking configuration and add new options for finer control over what SDK code is included in the final bundle.
The most significant changes are:
Closes #18417