Make FirebaseConfig properties available as internal parameters #4970
Conversation
Codecov ReportBase: 56.93% // Head: 56.96% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #4970 +/- ##
==========================================
+ Coverage 56.93% 56.96% +0.03%
==========================================
Files 292 292
Lines 19476 19483 +7
Branches 3898 3900 +2
==========================================
+ Hits 11088 11098 +10
+ Misses 7451 7450 -1
+ Partials 937 935 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
src/deploy/functions/params.ts
Outdated
| defaultParams["DATABASE_URL"] = new ParamValue(config.databaseURL, true, { | ||
| string: true, | ||
| boolean: false, | ||
| number: false, | ||
| }); | ||
| defaultParams["PROJECT_ID"] = new ParamValue(config.projectId, true, { | ||
| string: true, | ||
| boolean: false, | ||
| number: false, | ||
| }); | ||
| defaultParams["GCLOUD_PROJECT"] = new ParamValue(config.projectId, true, { | ||
| string: true, | ||
| boolean: false, | ||
| number: false, | ||
| }); | ||
| defaultParams["STORAGE_BUCKET"] = new ParamValue(config.storageBucket, true, { | ||
| string: true, | ||
| boolean: false, | ||
| number: false, |
There was a problem hiding this comment.
I'm pretty sure our type for FirebaseConfig is wrong. databaseURL and storageBucket may be empty for a project that didn't initialize neither of these products. Should we instead conditionally setup those parameters?
src/deploy/functions/params.ts
Outdated
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
add documentation or remove?
| boolean: false, | ||
| number: false, | ||
| }); | ||
| } | ||
| defaultParams["PROJECT_ID"] = new ParamValue(config.projectId, true, { |
There was a problem hiding this comment.
nit* I wonder if we need PROJECT_ID. We reserve GCLOUD_PROJECT and not PROJECT_ID - what would happen if user had defined PROJECT_ID in the dotenv file 🤔
There was a problem hiding this comment.
The user's definition wins. I added a test case to make this clear.
Based on bugbash feedback from @taeold and @colerogers.
We repurpose the
.secretfield of ParamValue (which wasn't being used anyway after we washed our hands of handling Secrets ourselves) to be.internal, which designates a param that we generated which shouldn't be written to disk or the runtime process.env.We make the fields of FirebaseConfig--project id, RTDB URL, and storage bucket--available as internal params to the resolution process.
In order for this to do anything useful, the SDK will have to export corresponding special Expressions that users can use in their Functions; these Expressions will not add anything to the manifest (since they'll be generated by this code anyway) but will have to have their .value() function special cased to read from process.env.firebase_config instead.