-
Notifications
You must be signed in to change notification settings - Fork 937
Component Framework #2378
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
Component Framework #2378
Conversation
* Component implementation * update dep version * [AUTOMATED]: Prettier Code Styling * [AUTOMATED]: License Headers * rename variables * address comments * [AUTOMATED]: Prettier Code Styling * remove unused comment * update code * [AUTOMATED]: Prettier Code Styling * rename to clearInstance to have naming consistency * make FirebaseApp tests work again * fix node tests * [AUTOMATED]: Prettier Code Styling * add comments for ComponentType * [AUTOMATED]: Prettier Code Styling * pass Component directly into Providers * [AUTOMATED]: Prettier Code Styling * correct spellings * update readme * fix lint issue * remove unused import * fix API change * move types around * [AUTOMATED]: Prettier Code Styling * improve provider typing * [AUTOMATED]: Prettier Code Styling * Migrate analytics to component platform (#220) * migrate analytics * minor analytics refactoring * interface merge * [AUTOMATED]: Prettier Code Styling * [AUTOMATED]: License Headers * allow overwriting a registered component * [AUTOMATED]: Prettier Code Styling * change ComponentType to string enum * address comments * [AUTOMATED]: Prettier Code Styling * remove return only generics * Move identifier to options object for getImmediate() * [AUTOMATED]: Prettier Code Styling * Make getProvider() type safe * [AUTOMATED]: Prettier Code Styling * define a new method to replace overwrite flag * [AUTOMATED]: Prettier Code Styling * Make component type safe * [AUTOMATED]: Prettier Code Styling * remove the generic type from component container * Update FirebaseApp and Analytics * [AUTOMATED]: Prettier Code Styling * remove unneccessary casting * [AUTOMATED]: Prettier Code Styling * fix typo * address comments * [AUTOMATED]: Prettier Code Styling * update some types * [AUTOMATED]: Prettier Code Styling * handle errors from instance factory * [AUTOMATED]: Prettier Code Styling
* Migrate Performance to component framework * [AUTOMATED]: Prettier Code Styling * removed unused import
* Migrate RC to component framework * [AUTOMATED]: Prettier Code Styling * remove unused import
* Migrate Database to component framework * revert auth formatting
* Migrate installations to component framework * remove unused import * update calling code according to the latest component changes * added forceRefresh back
* Migrate messaging to component framework * remove unused import * bundle firebase services in a single object * [AUTOMATED]: Prettier Code Styling * [AUTOMATED]: License Headers * address comment
* Migrate Firestore to component framework * [AUTOMATED]: Prettier Code Styling * remove unused import * removed unnecessary type assertion * update getImmeidate call
* update database types * update Firestore types * update installations type * update messaging types * update functions types * update performance types * update remoteconfig types * update storage types * fix analytics issue * [AUTOMATED]: Prettier Code Styling
* Mirgrate Auth to component framework * update Auth types * [AUTOMATED]: Prettier Code Styling * address comments
* Get installations from the container * [AUTOMATED]: Prettier Code Styling * revert dev changes * convert eslint from json to js. * [AUTOMATED]: Prettier Code Styling * fix broken scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed Installations and Messaging.
message_id: data[FN_CAMPAIGN_ID], | ||
message_time: data[FN_CAMPAIGN_TIME], | ||
message_device_time: Math.floor(Date.now() / 1000) | ||
this.services.analyticsProvider.get().then( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make the event listener async
and use await
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
export function makeFakeInstallations(): FirebaseInstallations { | ||
return { | ||
getId: () => Promise.resolve('FID'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very much a nit, but you can do getId: async () => 'FID',
. Also the other ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Changed here and in other places where we return a promise.
_NOTE: This is specifically tailored for Firebase JS SDK usage, if you are not a | ||
member of the Firebase team, please avoid using this package_ | ||
|
||
## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we get rid of the rest of the instructions from here on down if nobody should be installing it anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, but I think it's harmless to keep it, and it's consistent with other packages, e.g. @firebase/util
.
Adopt the component framework to manage SDK registration and inter SDK dependencies.
Design doc: go/firebase-js-platform