-
Notifications
You must be signed in to change notification settings - Fork 511
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
feat: add firebase functions v2 preset #1142
Closed
Closed
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
b2c1fde
Add the preset
JamieCurnow baabcd2
update the docs
JamieCurnow 72465c7
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow e4ca149
updated ts-ignore to ts-expect-error with comment.
JamieCurnow 9679aa1
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow a61a507
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow b97e832
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow f8ed16e
Adding the firebase v2 options to the PresetOptions interface
JamieCurnow cbbc21d
Documentation for the firebaseV2 options as part of nitro config
JamieCurnow 0a929c4
Merge branch 'main' into feat/firebase-functions-v2 and fix pnpm lockβ¦
JamieCurnow 07a4d4b
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow 7266ac1
re-sync pnpm-lock with main
JamieCurnow 9d1ff24
fresh pnpm install to get firebase-admin in
JamieCurnow da30242
clean up hanging comment
JamieCurnow ca0e1ed
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow 52fbf62
update firebase v2 link from docs
JamieCurnow dd2d2a1
update httpRequestOptions link in docs
JamieCurnow a4e703e
change firebase import to specify that it's a type
JamieCurnow 7990f8f
move firebase functions dep to dev deps
JamieCurnow 7163338
rename preset to firebase-v2
JamieCurnow 41fbaa5
chore: move firebase functions to deps
Hebilicious 5746f46
add warning to update firebase tools cli when using firebase-v2
JamieCurnow 01f3e4e
updates to the firebase docs to be explicit about installing firebaseβ¦
JamieCurnow 5f53f9f
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow bd29d15
fix firebase v2 http options passing through from nitro config
JamieCurnow 37b400d
tidy docs
JamieCurnow 6711bb9
Add message about non-public functions
JamieCurnow 424fe2d
Change docs to recommend using npx firebase-tools
JamieCurnow d4ea814
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow efc5bd0
nest httpRequestOptions under the v2 key in the firebase preset options
JamieCurnow 112caa4
Update docs/content/2.deploy/providers/firebase.md
Hebilicious 8797fbc
update nitro entry build command to be more correct
JamieCurnow 881d932
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow 9bf2f3f
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow ac61871
Merge main and pnpm install to fix lock file
JamieCurnow 937c6df
import only the onRequest function for firebase v2 entry
JamieCurnow 4c4a81b
Merge main and pnpm install to update lock file
JamieCurnow a22b4f1
Merge branch 'main' into feat/firebase-functions-v2
JamieCurnow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think it's better to name this something different to keep the
firebase
config option for thefirebase
preset only. It will avoid any possible conflicts.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.
I'll let @pi0 weigh in on this, as we've already been through a review on it. See the resolved issue here @posva:
#1142 (review)
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.
Just my 2 cents, but for #1377 my idea was to have the config option shared between v1 and v2 functions, so you could seamlessly switch between them without worrying about the config. As someone using this in production, it would be nice to switch between v1 and v2 without worrying about accidentally overwriting function deployments.
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.
@AlexLavoie42 Why would you switch between 1st and 2nd gen in production? Normally one would upgrade to gen 2 because they are better but they would never switch back. Even more when talking about the function that handles SSR. Or are you talking about upgrading too? In that case, I also think adding the same options where possible would make sense
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.
You're right, the vast majority of the time it will be just upgrading from v1 to v2, although I could see some circumstances where we would downgrade to v1 temporarily for debugging/testing purposes.
The main purpose behind sharing the config would be to simply upgrade to v2 without changing config. I have not actually tested this, but I assume publishing a v1 and v2 function with the same name will overwrite each other? If the config is separated there is definitely a possibility that when upgrading users will forget to move the config over and overwrite the default
server
function.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.
Could you detail those circumstances? I don't think I see any reason not to upgrade the currently named
server
function that renders the Nuxt app to gen 2 and then rollback to gen 1 π€Publishing a gen 2 function with the same name will error, you have to name it differently
Totally π
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.
This is good to know, so in that case you would not be able upgrade to v2 with the same function name unless you delete the original first? Perhaps in that case it should actually be a different config value to avoid an error. Maybe even the default name should be different as well then?
As for downgrading to v1, I am imagining a case where we downgrade temporarily either to debug or rollback in case of any bugs that arise after upgrading. Granted, I don't think this scenario is very likely.
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.
IMO the name should be different yes.
Not exactly: upgrading is a bit different as you want to go through a one-time step where you have both gen functions co-existing. In more complex scenarios: https://firebase.google.com/docs/functions/2nd-gen-upgrade.
You could also just redeploy the second gen with a different name and remove the 1st one in one deployment. I'm not completely sure but I imagine you risk losing a fraction of traffic during that deployment, which in some projects is fine, in others isn't.