-
Notifications
You must be signed in to change notification settings - Fork 10.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
fix(gatsby): Set CORS to * in serve #14483
fix(gatsby): Set CORS to * in serve #14483
Conversation
I don't have the slightest idea what to do about this Danger failure. |
This might be unnecesary or changed after #14443 - will do some investigation.
This is flake - somethings Peril/danger (tools we use for some workflows automation) errors out randomly with confusing message. You can ignore it |
Ok. let's not remove cors entirely - but with #14443 we will be able to use wildcard for allowed origins (and remove allow-credentials). |
This reverts commit 3ace166.
This is blocked until #14443 is merged. |
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.
This can go in now!
Thank you so much 👍
Holy buckets, @stutrek — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! |
I'm a long time user, first time contributor. Thanks for the great framework, the development experience for using and editing Gatsby is incredible.
Description
Access control headers were added to the server about a month ago, using the host provided from the command line. I am running the server with
gatsby serve --host 0.0.0.0 --port 80
because it's being served on multiple domains, with routing and https handled before Gatsby. Currently these headers are served:Unless I'm misunderstanding something, access control headers aren't useful in this situation because they'll always be the domain the browser is already on (with the exception of my 0.0.0.0 scenario). These headers were added here as part of #12128 so there may be a situation with
assetPrefix
on another domain that I'm unaware of. @DSchau can you confirm or deny? Should this be a CLI flag when running the server insetad?I might not be running the tests properly, no tests failed after this change.
Related Issues
Introduced by #12128