-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
docs(nextjs): added tree-shaking webpack config guide #15790
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
c5f16a6 to
6b6c635
Compare
inventarSarah
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.
👍
Added some suggestions
|
|
||
| <SdkOption name="webpack.treeshake.removeDebugLogging" type="boolean" defaultValue="false"> | ||
|
|
||
| Setting this option to `true` will remove all debug logging code from the Sentry SDK. Note that it has nothing to do with `enableLogs` or Sentry's logs product. |
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.
These two sentences are a bit confusing to me -- which logs does it concern?
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.
It's a bit tricky 😅, so ... we have those DEBUG logs that show up when the user has debug: true but they aren't removed from the bundle.
So this option is supposed to remove them from the bundle, but we have a logs product enabled by enableLogs on the SDK configuration (not build options), so this is basically trying to avoid that very confusion, that it doesn't affect the logs product and it only affect those DEBUG statements.
What do you think we could do here to explain that? this has been always a confusion point for many users.
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.
Something like this, maybe?
Setting this option to true will remove all Sentry SDK debug logging code (the console logs that appear when you set debug: true in your SDK configuration). This doesn't affect Sentry's Logs product (controlled by the enableLogs option) or your app's logging.
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.
Yea, I think the console mention here nails it. Thanks! Pushed it!
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
inventarSarah
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.
💫nice!
docs/platforms/javascript/guides/nextjs/configuration/tree-shaking.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Sarah Mischinger <sarah@codingwriter.com>
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.treeshakenamespace. It has a few differences to the existing tree-shaking flags under the hood:trueto take effect, unlike the flags where some had to be set totrueand some had to be set tofalse.webpack.treeshakebuild 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.