-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
client/web: add support for zst precomppressed assets #12170
base: main
Are you sure you want to change the base?
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.
Tested locally to make sure things all still work okay
c6cfb56
to
5071bbb
Compare
I updated some more to do the build side, and tested against chrome with a local workspace setup, updating the build in the prebuilt repo locally. I caught a window size limit error with chrome, and that is also now fixed, so these are serving successfully now with precompressed zstd. |
211b9ca
to
a9637fd
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.
LGTM, once this gets in, we can run the following two actions (in this order) to update the prebuilt assets then make sure everything looks okay:
d1f3ef8
to
6bbddf3
Compare
This will enable us to reduce the size of these embedded assets. Updates tailscale/corp#20099 Signed-off-by: James Tucker <james@tailscale.com>
6bbddf3
to
f39ac7d
Compare
return f, nil | ||
} | ||
} | ||
if tswebutil.AcceptsEncoding(r, "br") { |
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 it's time to drop Brotli to make up for the increased cost/size of adding zstd?
Safari users can use gzip for now?
This will enable us to reduce the size of these embedded assets.
Updates tailscale/corp#20099