Support Params as inputs in eventTrigger #168
Conversation
The api of the manifest changed in firebase-functions@3.23. Some inputs have been updated to be `T | Expression<T>`.
| @@ -47,7 +47,7 @@ | |||
| "@types/mocha": "^5.2.7", | |||
| "chai": "^4.2.0", | |||
| "firebase-admin": "^10.1.0", | |||
| "firebase-functions": "^3.22.0", | |||
| "firebase-functions": "^3.23.0", | |||
There was a problem hiding this comment.
Should we update the peer-depdency? It looks like our package won't work unless you are using firebase-functions 3.23 or above now since we explicitly try to import the Expression type in v2/params.
There was a problem hiding this comment.
You're extremely right!!!
There was a problem hiding this comment.
That probably means we need to do a minor or major bump then... WDYT?
| @@ -1,4 +1,5 @@ | |||
| import { CloudEvent, CloudFunction } from 'firebase-functions/v2'; | |||
| import { Expression } from 'firebase-functions/v2/params'; | |||
There was a problem hiding this comment.
Aside - we are planning to change the import path of Expression from v2/params to just /params.
I wonder if there are more "elegant' way to import the type to avoid having this problem 🤔
There was a problem hiding this comment.
When is the change going to occur? 🙃
| if (typeof stringOrExpression === 'string') { | ||
| return stringOrExpression; | ||
| } | ||
| return stringOrExpression?.value(); |
There was a problem hiding this comment.
Probably double check with @Berlioz - but I thought it is very possible that this to return empty string.
I'm wondering if we should think more about supporting Params firebase-functions.
Params are a way to specify "value of this is going to be determined at deploy time" - so it is probably going to be undefined at test time unless there is some special override mechanism.
There was a problem hiding this comment.
The problem I was working against was eventTrigger.eventFilters and eventTrigger.eventFilterPathPatterns were converted to type Record<string, string | Expression<string>> :|
There was a problem hiding this comment.
The tests show a scenario where its populated at runtime too
The api of the manifest changed in firebase-functions@3.23. Some inputs have been updated to be
T | Expression<T>.Description
The api of the manifest changed in firebase-functions@3.23. Some inputs have been updated to be
T | Expression<T>.Code sample