-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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: Handle zstd encoding in webpack proxy config #30034
Conversation
THANK YOU! |
Also big thanks from me! I'd be curious to understand what caused this regression.. |
Closing #29822 as superseded by this one. |
@@ -201,6 +201,7 @@ | |||
"rimraf": "^6.0.1", | |||
"rison": "^0.1.1", | |||
"scroll-into-view-if-needed": "^3.1.0", | |||
"simple-zstd": "^1.4.2", |
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.
@kgabryje shouldn't this be in devDependencies
?
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 should, I moved it to dependencies to silence eslint errors. It shouldn't really affect the entry point sizes though, as webpack will put this package in a chunk that's not downloaded on any page
Having issues on master RN, saying In
|
I put |
I don't understand why it's not installed while it's specified here https://github.com/apache/superset/blob/master/Dockerfile#L35, debugging RN and had a |
Is it possible that docker compose skips the part where we install zstd in existing containers? |
idk - was messing around with it and now exact same SHA command and now magically works, feels like there was a corrupt binary somewhere at Debian and now it's fixed. |
This is happening to me... any idea what the magic fix could have been? I've tried nuking all existing and cached containers. |
I managed to "fix" it by hacking in here: https://github.com/apache/superset/blob/master/superset-frontend/package.json#L49 |
Really shouldn't be problem, wondering if nuking docker caches and |
SUMMARY
Some of the latest changes caused the
localhost:9000
to use zstd encoding, which we didn't handle inwebpack.proxy-config.js
. This PR addssimple-zstd
lib to decompress zstd encoded data.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
Run
npm run dev-server
, go tolocalhost:9000
, verify that it works normallyADDITIONAL INFORMATION