Fix messaging being optional and isSupported type definitions #793
Conversation
mmermerkaya
force-pushed
the
mmermerkaya-fix-types
branch
2 times, most recently
from
edc22aa to
56dba41
Compare
| messaging?: { | ||
| (app?: FirebaseApp): types.FirebaseMessaging; | ||
| Messaging: typeof types.FirebaseMessaging; | ||
| messaging: { |
There was a problem hiding this comment.
Soo... @firebase/messaging is optional; you can use @firebase/database etc. but not messaging just fine.
firebase the metapackage bundles everything together, cool.
But if you just depend on @firebase/messaging, it would monkey-patch @firebase/app-types to add itself optionally? weird
There was a problem hiding this comment.
They are all optional (you can import them or not), but if you import them, then the namespace exists (and the type should not be "optional" anymore). I'm not sure why that ? ever existed, I'm hoping @jshcrowthe has an idea.
| import { | ||
| _FirebaseNamespace, | ||
| FirebaseServiceFactory | ||
| } from '@firebase/app-types/private'; |
There was a problem hiding this comment.
It'd be really nice to have a test that works like the docs suggests a user to, i.e.
import firebase from '@firebase/app';
import '@firebase/messaging'
// Do stuff w/ `firebase` and `firebase.messaging`
rather than poking at the internals like this :/
There was a problem hiding this comment.
Yeah, I agree. The tests need an overhaul (not just here). I'll get to that soon™.
Fixes firebase.messaging() method being optional in TS types. Modifies isSupported so it is a namespace export and not a property on the factory function, which is not actually exported.
mmermerkaya
force-pushed
the
mmermerkaya-fix-types
branch
3 times, most recently
from
9e1a304 to
4196fab
Compare
Fixes TS thinking
firebase.messaging()method is optional after importing 'firebase/messaging' (removes the?). This should be done in other packages as well, unless there's a reason for the discrepancy between importing from'firebase'and from'firebase/packageName'that I don't know about.Modifies
messaging.isSupported()so it is a namespace export and not a property on the factory function, which is not actually exported. Fixes types accordingly.