-
Notifications
You must be signed in to change notification settings - Fork 143
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
dont't fallback to index page if browser doesn't want html #60
Comments
So you are proposing to remove |
Yes,yes! Most type of auto-sent requests will accept
If we keep |
I am well aware of the theory behind this, but so far this library seems to have worked very well for its users. This is why I asked for an actually existing problem caused by this default configuration option. Given a download rate of currently over 4 million times per week, I am not willing to make such a potentially breaking change and through that affect users. |
any workaround for this? my issue is described here shellscape/webpack-plugin-serve#130 it only happens when disableDotRule is enabled |
@bripkens this is a real issue affecting users. It's wonderful that the module has so many downloads! But that doesn't discount the effect of a small bug when one is found. Breaking changes are tough; they require documentation, a major release, and lots of questions from users. Wouldn't a more reasonable solution then be an option to allow for different behavior to resolve this case? That would only mean a minor version, minimal documentation, and no effect to users who didn't want to change the original behavior. You could even ask for feedback in the future on whether or not users on the whole would like to see the option become the default. |
Sounds like a fair approach @shellscape. Also, I would very much appreciate a sample app under |
Thank you @bripkens @sibelius @fuweichin can either of you take up the effort for this contribution? |
It's unreasonable to serve html for non-html requests with status code 2xx. so connect-history-api-fallback shouldn't redirect to index page if browsers don't explicitly accept html, thus
options.htmlAcceptHeaders
should default to["text/html"]
from["text/html","*/*"]
since most dependents(at least those I 'm using) and developers don't set that option and most browsers' request header 'accept' contains "text/html"See also my history-api-fallback solution for Nginx React-router and nginx
The text was updated successfully, but these errors were encountered: