Skip to content

Conversation

@mvayngrib
Copy link

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 :)

@maciejtreder maciejtreder self-requested a review November 3, 2017 13:03
@maciejtreder
Copy link
Owner

@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!

@mvayngrib
Copy link
Author

todo: load the cors config from the function's cors config

@stormit-vn
Copy link

We really need this feature in place, so anyone can help to get this PR merged?

Thank in advance!

@maciejtreder
Copy link
Owner

@mvayngrib Could you resolve conflicts with the master branch, so we could go forward with merge?

@mvayngrib
Copy link
Author

@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 :)

@maciejtreder
Copy link
Owner

@mvayngrib Ouch... You're right..

Will you find time to implement CORS configuration rewrite/copy?

@mvayngrib
Copy link
Author

@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?

@rcstr
Copy link

rcstr commented Mar 26, 2018

any update on this? what's missing? anything I can help with?

@mvayngrib
Copy link
Author

@rcstr yep! Read above about CORS

@Ponjimon
Copy link

Hello,

is there any status update here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants