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): support renaming exported server function #1377

Merged
merged 1 commit into from
Aug 20, 2023

Conversation

AlexLavoie42
Copy link
Contributor

@AlexLavoie42 AlexLavoie42 commented Jul 2, 2023

πŸ”— Linked issue

#266

❓ 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

Adds a new option to nitro config that allows giving a custom name to deployed firebase functions.

This is done using rollup replace. I changed the exported name for the server in the firebase entry to __firebaseServerFunctionName__ to ensure it is unique to the firebase server export. The placeholder name is then replaced with firebase.serverFunctionName in the config, which is given a default value of "server".

This change gives an intuitive way to deploy multiple servers to the same firebase project without having them be overwritten. The workaround given in #266 is not very intuitive, and could potentially break other deployments in the future.

I would also recommend adding the same option to V2 functions in #1142, since there doesn't appear to be a way to change the deployed function name in the V2 HttpOptions (I don't think labels does this, but correct me if I am wrong)

Resolves #266

πŸ“ Checklist

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

I have tested building my own project and the function renaming appears to work as expected. The only difference is that export { s as server } from './chunks/nitro/firebase.mjs' becomes export { _ as server } from './chunks/nitro/firebase.mjs' since the export now starts with _ before the placeholder gets replaced. I don't think this will cause any issues, but I am not 100% sure.

@nuxt-studio
Copy link
Contributor

nuxt-studio bot commented Jul 2, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
nitro Edit on Studio β†—οΈŽ View Live Preview ae1cc96

@Hebilicious
Copy link
Contributor

This is looking good thanks ! Let's wait until #1142 is merged before moving forward with that one.

@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #1377 (7ddc037) into main (a04c663) will increase coverage by 0.05%.
The diff coverage is 85.71%.

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

@@            Coverage Diff             @@
##             main    #1377      +/-   ##
==========================================
+ Coverage   76.54%   76.60%   +0.05%     
==========================================
  Files          76       71       -5     
  Lines        7773     7248     -525     
  Branches      784      722      -62     
==========================================
- Hits         5950     5552     -398     
+ Misses       1821     1694     -127     
  Partials        2        2              
Files Changed Coverage Ξ”
src/rollup/config.ts 89.75% <50.00%> (-1.77%) ⬇️
src/options.ts 94.40% <100.00%> (-2.25%) ⬇️

... and 31 files with indirect coverage changes

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR πŸ’š I have rebased it and updated implementation to match the latest requirements

@pi0 pi0 changed the title feat(firebase): support renaming deployed firebase functions feat(firebase): support renaming exported server function Aug 20, 2023
@pi0 pi0 removed the pending label Aug 20, 2023
@pi0 pi0 merged commit 6cfdf01 into nitrojs:main Aug 20, 2023
6 checks passed
@pi0 pi0 mentioned this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request preset:firebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[firebase] support renaming functions
3 participants