-
Notifications
You must be signed in to change notification settings - Fork 33
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
get OPTIONS working #35
base: master
Are you sure you want to change the base?
Conversation
@mvayngrib Thanks for this PR! Yup, you're right. We definetely need tests here. I will try to add some, but I am afraid that it won't happen quickly (my backlog is really full for next 2 months). So if you will have any free time, please add those. Thanks i advance! |
todo: load the cors config from the function's cors config |
We really need this feature in place, so anyone can help to get this PR merged? Thank in advance! |
@mvayngrib Could you resolve conflicts with the master branch, so we could go forward with merge? |
@maciejtreder I updated the PR. Keep in mind that this is still missing copying over of the cors config, so if people have custom headers they want allowed, they will be sad :) |
@mvayngrib Ouch... You're right.. Will you find time to implement CORS configuration rewrite/copy? |
@maciejtreder i was trying to see the other day how serverless parses it out, and it's somewhere in the bowels of the code: https://github.com/serverless/serverless/blob/master/lib/plugins/aws/package/compile/events/apiGateway/lib/validate.js#L298 do you know how we might access that method? |
any update on this? what's missing? anything I can help with? |
@rcstr yep! Read above about CORS |
Hello, is there any status update here? |
uses the trick learned here:
CodeGenieApp/serverless-express#58
CodeGenieApp/serverless-express#58 (comment)
CodeGenieApp/serverless-express#58 (comment)
to solve the problem discussed here;
serverless/serverless#2797 (comment)
i also refactored the stage/apiId variables to be instance variables, they were a pain to pass around
i understand if you don't want to merge this for lack of tests, so either I'll get to it when I have time, or someone else will :)