-
Notifications
You must be signed in to change notification settings - Fork 201
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
Event context can be undefined according to type definitions #227
Comments
Could you copy and paste the exact line, and from which .d.ts you are referring to? |
Hi @laurenzlong, vcode points me to
onCreate(handler: (snapshot: DocumentSnapshot, context?: EventContext) => PromiseLike<any> | any): CloudFunction<DocumentSnapshot>; So here the context argument is optional, and as a result TSC forces you to check its existence inside the handler function. I think this would be better solved with an overload for onCreate to allow both 1 and 2 args handlers. |
@0x80 Thanks for the additional info. I'm not able to reproduce the issue on my end. Is it a linter error or an actually TypeScript compiler error you're getting? Do you get the error when you run "tsc"? What's your typescript version and tsconfig.json? |
It is a compiler error, and it should be given the signature I think. Maybe you didn't use strict compiler settings? I'm using Typescript 2.8 and this is my config: {
"compilerOptions": {
"target": "ES2016",
"module": "commonjs",
"lib": ["esnext"],
"sourceMap": true,
"outDir": "build",
"strict": true,
"noImplicitAny": true,
"strictNullChecks": true,
"strictFunctionTypes": true,
"noImplicitThis": true,
"alwaysStrict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"moduleResolution": "node",
"plugins": [
{
"name": "tslint-language-service"
}
]
},
"include": ["src/**/*"]
} |
It's just defined like this in the source code, unnecessarily AFAIK. I've made a PR yesterday. |
* Scheduler * unit tests * Adds integration tests for scheduled functions * removing uninented commit * fixing version number * adding labels and test for labels * trigger tests off of call to cloud scheduler:run * adding region/runtime opts tests * Predictions Functions (#220) * Firebase Predictions Integration with Functions - Initial check in * Add predictions getter * Make API match what was in the review, and add unit tests. * Fix imports. * Add error handling. * Handle process.env.GCLOUD_PROJECT correctly. * Fix region. * Upper case RiskToleranceName. * Add changelog message. * formatting * cleaning up * Revert "Predictions Functions (#220)" (#226) This reverts commit 838597aa6df973462a50484ec1f50d625da3f633. * pr fixes * adding clarifying comments * starting on pubsub impl * adds pusbub.schedule() * switching tests over from https to pubsub * Change timezone to a method instead of an optional argument * use real eventType * remove extra slash * changelog * switch signature to onRun(context) * switches changelog to present tense
Typescript version 3.2.2, latest firebase - functions getting the same lint errors while following documentation on firebase. :( |
Latest firebase, if we use |
According to the 1.0.0 Typescript definitions the context part of an event is defined as
EventContext | undefined
. For example:I have a hard time imagining why this would ever be undefined, so I am currently ignoring it.
Context as the second parameter to the handler function is optional, because you can supply a handler with just the
data
argument. But shouldn't that be solved via overloading? If I supply a function with both arguments then context should be guaranteed to exist, no?The text was updated successfully, but these errors were encountered: