-
Notifications
You must be signed in to change notification settings - Fork 79
Only intercept preflight requests for fonts, and bonus insert_target configuration option
#18
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
base: master
Are you sure you want to change the base?
Conversation
Don't assume this is the only middleware handling preflights.
|
+1 |
|
Could you expand this PR to include some specs to demonstrate the features working? To run the specs all you have to do is |
|
+1 Could use some merging |
|
Still looking for specs to prove this works before merging. |
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.
defined?(ActionDispatch::Static) never returns false, even if serve_static_assets is false. This should probably check for its presence in app.middleware instead.
|
+1, please merge -- this gem's current behavior of responding to any preflight request is really confusing. |
|
@dmur and @sandeep45, I can't merge this PR until there are specs proving this functionality works. It would be irresponsible of me to just merge in some code that is untested. |
|
@ericallam Understood. I'm not a rails guy by trade or I'd write the specs myself. |
Sorry this is a two in one
Rack::Corsor any other setup that also requires special preflight responsesinsert_targetoption so that you can choose which middleware you wantfont_assetsto be inserted before