Skip to content
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(firebase): add support for 2nd generation functions #1500

Merged
merged 48 commits into from
Aug 7, 2023

Conversation

posva
Copy link
Collaborator

@posva posva commented Jul 28, 2023

🔗 Linked issue

Forked from #1142

Resolves #1141
Resolves #666
Resolves #90

This PR adds a fireabse.gen option instead of firebase.v2 of the original PR to allow customizing gen 2 and gen 1 functions. It also allows passing other options for the functions. This assumes you are only using one generation at the same time (which, outside of migrations should be the case).

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@nuxt-studio
Copy link
Contributor

nuxt-studio bot commented Jul 28, 2023

Live Preview ready!

Name Edit Preview Latest Commit
nitro Edit on Studio ↗︎ View Live Preview a66ded5

@pi0 pi0 self-requested a review July 28, 2023 17:06
@posva posva changed the title Add support for 2nd generation functions to the firebase preset feat: add support for 2nd generation functions to the firebase preset Jul 28, 2023
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #1500 (cfcd2f5) into main (fc55550) will increase coverage by 0.03%.
The diff coverage is 36.36%.

❗ Current head cfcd2f5 differs from pull request most recent head a66ded5. Consider uploading reports for the commit a66ded5 to get more accurate results

@@            Coverage Diff             @@
##             main    #1500      +/-   ##
==========================================
+ Coverage   76.47%   76.50%   +0.03%     
==========================================
  Files          73       73              
  Lines        7591     7594       +3     
  Branches      757      757              
==========================================
+ Hits         5805     5810       +5     
+ Misses       1785     1783       -2     
  Partials        1        1              
Files Changed Coverage Δ
src/presets/firebase.ts 21.36% <36.36%> (+3.82%) ⬆️

package.json Outdated Show resolved Hide resolved
@posva posva marked this pull request as ready for review July 28, 2023 17:16
@posva
Copy link
Collaborator Author

posva commented Jul 28, 2023

@JamieCurnow It was really hard for me to create a code review with all of this so I created a fork of your PR while keeping all the commits. Let me know what you think 🙂

docs/content/2.deploy/providers/firebase.md Show resolved Hide resolved
docs/content/2.deploy/providers/firebase.md Outdated Show resolved Hide resolved
docs/content/2.deploy/providers/firebase.md Outdated Show resolved Hide resolved
docs/content/2.deploy/providers/firebase.md Outdated Show resolved Hide resolved
src/presets/firebase.ts Outdated Show resolved Hide resolved
@pi0 pi0 changed the title feat: add support for 2nd generation functions to the firebase preset feat(firebase): add support for 2nd generation functions Jul 28, 2023
@pi0 pi0 changed the title feat(firebase): add support for 2nd generation functions feat(firebase): add support for 2nd generation functions Jul 28, 2023
@JamieCurnow
Copy link
Contributor

@posva I've made a PR to your branch on your fork with the suggestions above:
posva#1

Hopefully this is the best way to do this? 😅

@pi0
Copy link
Member

pi0 commented Jul 28, 2023

Thanks for quick PR @JamieCurnow and all followup ❤️

Let's hope we get out of this situation soon and keep improving preset directly on one main branch 😅

PS: I have sent you an email regarding to join discord server to collaborate closely.

src/presets/firebase.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@JamieCurnow JamieCurnow left a comment

Choose a reason for hiding this comment

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

@posva Just adding some of the suggestions from @pi0 here. I've realised that we need to also update the doc changes now to account for the gen selection and to note that the node version can be pinned via the new nodeVersion key. I'm happy to make these changes - may be easier to add me as a collaborator to your fork? Otherwise, let me know and I'll make a PR to your fork again if you want me to jump on this 🙏

src/types/presets.ts Outdated Show resolved Hide resolved
src/presets/firebase.ts Outdated Show resolved Hide resolved
@posva posva force-pushed the feat/support-firebase-gen-two branch from ac52be4 to 328dab3 Compare July 31, 2023 13:49
docs/content/2.deploy/providers/firebase.md Outdated Show resolved Hide resolved
docs/content/2.deploy/providers/firebase.md Outdated Show resolved Hide resolved
src/options.ts Outdated Show resolved Hide resolved
src/options.ts Outdated Show resolved Hide resolved
@pi0 pi0 mentioned this pull request Aug 6, 2023
7 tasks
@pi0
Copy link
Member

pi0 commented Aug 7, 2023

Thanks you all for contributing on this preset ❤️ It looks amazing! To move forward i have made some minor refactors as well as docs update about development. Feel free to try it and make PRs directly into the main branch.

BTW we also need to add local tests

@pi0 pi0 merged commit 99fbfb9 into nitrojs:main Aug 7, 2023
6 checks passed
@snsxn
Copy link

snsxn commented Aug 11, 2023

Hi!
I see in the Nitro docs the new 2nd gen functions, region and node version.
is it released?

Thx!!

@pi0
Copy link
Member

pi0 commented Aug 11, 2023

Hi dear @snsxn. you can try with edge channel: https://nitro.unjs.io/guide/getting-started#edge-release-channel

@pi0 pi0 mentioned this pull request Aug 21, 2023
@husayt
Copy link

husayt commented Dec 4, 2023

when running firebase emulator it seems is missing region parameter.

Here is the suggestion which could help:
https://stackoverflow.com/questions/72339840/firebase-emulator-cloud-functions-only-works-with-default-us-central1-location-i

FirebaseFunctions.instanceFor(region: 'europe-west2').useFunctionsEmulator('localhost', 5001);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants