-
Notifications
You must be signed in to change notification settings - Fork 36
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
Transport minified version of query. #277
Conversation
e7a4152
to
7c88078
Compare
Job #133: Bundle Size — 1.01MiB (~+0.01%).Warning Bundle contains 1 duplicate package – View duplicate packages Bundle metrics
|
Current Job #133 |
Baseline Job #115 |
|
---|---|---|
Initial JS | 890.63KiB (~+0.01% ) |
890.58KiB |
Initial CSS | 0B |
0B |
Cache Invalidation | 1.5% |
0% |
Chunks | 24 |
24 |
Assets | 45 |
45 |
Modules | 512 |
512 |
Duplicate Modules | 30 |
30 |
Duplicate Code | 1.29% |
1.29% |
Packages | 29 |
29 |
Duplicate Packages | 1 |
1 |
Bundle size by type 1 change
1 regression
Current Job #133 |
Baseline Job #115 |
|
---|---|---|
JS | 1023.89KiB (~+0.01% ) |
1023.84KiB |
Other | 5.99KiB |
5.99KiB |
View job #133 report View pr/transport-minified-queries branch activity View project dashboard
print(query) | ||
// replace multi-spaces with single space | ||
.replace(/\s{2,}/g, " ") | ||
// remove spaces that are preceeded by braces | ||
.replace(/(?<=[{}])\s+/g, "") | ||
// remove spaces that are preceeding braces | ||
.replace(/\s+(?=[{}])/g, "") | ||
.trim() |
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 looks like graphql
has a stripIgnoredCharacters
utility that handles this. Can we use that function to handle stripping the whitespace? It looks like that utility is lexer-aware so it should be a bit more accurate.
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.
Hmm, that pulls in about 10kb more bundle size - but since we're in SSR world here, that should be okay.
We might at some point get to the point that we pull in too much for edge functions, though (they have a limit of 1MB I believe?), so I'll make a note that that's a great place for a future size optimization.
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.
Hmm, that pulls in about 10kb more bundle size - but since we're in SSR world here, that should be okay.
We might at some point get to the point that we pull in too much for edge functions, though (they have a limit of 1MB I believe?), so I'll make a note that that's a great place for a future size optimization.
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.
Ah forget about it, most of that is included in gql
already, so it's likely fine anyways.
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.
My only request would be to use stripIgnoredCharacters
, but otherwise this looks great!
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Right now, we transport the AST of a query over the network.
That's not optimal:
Normal Query:
is instead transported as
and instead we could go smaller and transport