Skip to content

Conversation

stfsy
Copy link
Contributor

@stfsy stfsy commented Aug 12, 2022

Description

As discussed in firebase/firebase-tools#4862 it's not possible to disable cors in v2 functions when running in the emulator, because the emulator enables the debug feature for cors overriding opts.cors.

If we could prioritize the boolean flag over the debug feature, we could have both: The ability to disable cors and also prioritize the debug flag over user provided configuration.

Code sample

@stfsy
Copy link
Contributor Author

stfsy commented Aug 25, 2022

@taeold updated as per suggestion and rebased

Copy link
Contributor

@taeold taeold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - please fix formatting!

Thank you for an awesome contribution!

@stfsy
Copy link
Contributor Author

stfsy commented Sep 11, 2022

@taeold anything else I can do?

@stfsy
Copy link
Contributor Author

stfsy commented Sep 11, 2022

@taeold got it now, had to run npm run format:fix to fix the formatting error. I committed and pushed the changes just a few seconds ago.

stfsy and others added 5 commits September 16, 2022 19:46
As discussed in firebase/firebase-tools#4862 it's not possible to disable cors in v2 functions, because the emulator enables the debug feature for cors overriding `opts.cors`.

If we could prioritize the boolean flag over the debug feature, we could have both: The ability to disable cors and also prioritize the debug flag over user provided configuration.
Co-authored-by: Daniel Lee <taeold@gmail.com>
@colerogers
Copy link
Contributor

@stfsy I fixed your merge conflicts but it looks like you need to run npm run format one more time before we can merge this.

@stfsy
Copy link
Contributor Author

stfsy commented Dec 15, 2022

@colerogers thank you very much 🙂 I ran npm run format and commited+pushed the results

@colerogers colerogers merged commit 27ce234 into firebase:master Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants