-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat(codecov): bundle analyzer #7829
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
Conversation
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 pull request integrates the Codecov bundle analyzer into our CI pipeline.
- Added the "@codecov/nextjs-webpack-plugin" dev dependency.
- Updated the Next.js configuration to enable the Codecov plugin when the ANALYZE environment variable is set.
- Modified the CI workflow to conditionally set the ANALYZE flag for supported OS and event types.
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
apps/site/package.json | Added Codecov bundle analyzer plugin as a dev dependency. |
apps/site/next.config.mjs | Integrated the Codecov plugin into the webpack configuration. |
.github/workflows/build.yml | Updated the build workflow to set the ANALYZE environment variable. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #7829 +/- ##
==========================================
- Coverage 75.48% 75.44% -0.04%
==========================================
Files 101 101
Lines 8309 8309
Branches 218 218
==========================================
- Hits 6272 6269 -3
- Misses 2035 2038 +3
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Hm, isn't this going to prevent us migrating to turbopack? |
How so? |
This is going to force Next.js to use Webpack, presumably? |
It doesn't force Next.js to use Webpack. If it's built with turbo, the |
Ah okay, so we'll just lose bundle reporting if we do migrate, but it doesn't outright block it 👍 |
Drafting while I verify this works, and see if there is a way to do bundle analysis through Codecov for both Turbopack and Webpack. |
This PR's bundle is available at http://app.codecov.io/github/nodejs/nodejs.org/bundles/feat%2Fcodecov-bundle-analysis |
@@ -96,6 +97,23 @@ const nextConfig = { | |||
'shiki', | |||
], | |||
}, | |||
webpack: (config, options) => { |
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.
Does this work even with TurboPack enabled? We are by default building on CI with TurboPack... So I imagine this wouldn't work?
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.
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.
Welp, apparently you passing the webpack config flag is disabling turbopack :/
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.
Huh https://github.com/nodejs/nodejs.org/blob/main/apps/site/package.json#L6 did someone remove the --turbo
flag?
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.
Was removed as part of the Cloudflare adapter PR
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.
Maybe that should be re enabled? I'll adjust this PR accordingly if that flag is added back
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.
Drafting while I adjust for turbo pack, plus this shouldn’t land until the flag is re added. |
There doesn't appear to be an elegant solution to this. We could write our own reporter, but that's more unrealted code we need to maintain. I'm closing this to give someone else the opportunity to find a solution. We need a reporter that supports both Turbopack and the App Router. |
Adds codecov bundle analysis to our CI