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): Restore tree shaking capabilities #7710

Merged
merged 1 commit into from Apr 3, 2023

Conversation

lforst
Copy link
Member

@lforst lforst commented Apr 3, 2023

We messed up in #6592 and destroyed tree shaking capabilities for webpack by entirely removing the sideEffects field in the package.json. This PR adds it back so tree shaking should be possible again.

Hopefully fixes #7680

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.66 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified) 64.52 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 19.2 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified) 56.89 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 21.57 KB (0%)
@sentry/browser - Webpack (minified) 72.11 KB (0%)
@sentry/react - Webpack (gzipped + minified) 21.59 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 52 KB (-0.18% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 28.26 KB (0%)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.45 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 44.78 KB (0%)
@sentry/replay - Webpack (gzipped + minified) 38.86 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 63.5 KB (0%)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 56.53 KB (0%)

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Can we test this to make sure we don't regress?

Also did you validate with a local app? Can you show before/after in the PR?

@lforst
Copy link
Member Author

lforst commented Apr 3, 2023

Can we test this to make sure we don't regress?

We already have tests in place for this! E2E tests would fail if this would eliminate side effects somehow.

Also did you validate with a local app? Can you show before/after in the PR?

Well, I just tried it out locally, and turns out replay does properly tree shake away in my Next.js 13 test application, regardless of this change. Maybe it is related to older webpack versions.

I am still going to go forward with this because it is more correct this way.

@lforst lforst marked this pull request as ready for review April 3, 2023 11:28
@lforst lforst merged commit e99492a into develop Apr 3, 2023
33 checks passed
@lforst lforst deleted the lforst-fix-nextjs-treeshaking branch April 3, 2023 11:29
@rchl
Copy link
Contributor

rchl commented Apr 3, 2023

I've crated somewhat relevant #7713

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.

@sentry/nextjs bundle size and tree shaking
4 participants