-
Notifications
You must be signed in to change notification settings - Fork 4k
Reduce entry bundle size - Part 5 #13128
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
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📉 Significant bundle size change detected
Please verify that this change is expected. |
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 reduces the frontend entry bundle size by optimizing the protobuf code generation process. The main optimization adds --no-verify and --no-delimited flags to the pbjs command, which removes unused verification and delimited encoding functions from the generated proto.js file. This achieves a 24% reduction in proto.js size (from 723 KB to 550 KB), resulting in an overall bundle size reduction from 2.15 MB to 1.99 MB.
Key Changes:
- Added optimization flags (
--no-verify,--no-delimited) to pbjs command in the protobuf generation script - Added
protobufjsas a devDependency in the protobuf package - Enhanced the generation script with a postProcess parameter for potential future optimizations
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/protobuf/scripts/generate-proto.js | Added --no-verify and --no-delimited flags to pbjs command; added unused postProcess parameter to runCommand function; updated console messages with emojis |
| frontend/protobuf/package.json | Added protobufjs ^7.5.4 as a devDependency |
| frontend/yarn.lock | Updated yarn.lock with new dependency resolutions including protobufjs, @types/node versions, long package, and undici-types |
📈 Frontend coverage change detectedThe frontend unit test (vitest) coverage has increased by 0.0200%
✅ Coverage change is within normal range. |
cc1d425 to
b273641
Compare
6b561b7 to
a7ec894
Compare
b273641 to
fc41261
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
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
frontend/connection/src/DoInitPings.tsx:229
- The status === 0 check may now be unreachable code with native fetch. Unlike axios, the native fetch API throws a TypeError for CORS errors and network failures rather than returning a response with status 0. These errors would be caught by the
error.isNetworkErrorcheck above (lines 255-271) instead.
While a test exists in utils.test.ts acknowledging this edge case is "rare" with native fetch, consider whether this branch still serves a purpose or if it should be removed to avoid confusion. If keeping it for defensive programming, consider adding a comment explaining why this unlikely case is handled.
if (status === /* NO RESPONSE */ 0) {
if (tooManyRetries) {
LOG.error(
`Client Error: response received with status ${status} when attempting to reach ${source}`
)
sendClientError(
`Response received with status ${status}`,
statusText,
source
)
}
return retryWhenTheresNoResponse()
}
lukasmasuch
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 👍
fc41261 to
b019017
Compare
a7ec894 to
930a516
Compare

Describe your changes
Attempt to improve initial load time by reducing dependencies included in entry bundle.
Changes made:
protobuf/proto.jssize by optimizing flags passed topbjs--no-verify&--no-delimitedflags to remove unused verification and delimited encoding functionsproto.jsfrom 723 KB to 550 KB (24%)axiosfrom entry bundledoInitPingsto usefetch, lazy loadaxiosinDefaultStreamlitEndpointswhen neededBundle Analysis:
Before (left): 2.36 MB & After (right): 2.16 MB
