Deprecated old Core configuration method #3147
Conversation
| /// Implement this method if the library needs notifications for lifecycle events. This method is | ||
| /// called when the developer calls `FirebaseApp.configure()`. | ||
| + (void)configureWithApp:(FIRApp *)app; | ||
| + (void)configureWithApp:(FIRApp *)app |
There was a problem hiding this comment.
I'm fairly certain this isn't used anymore, but we can leave it for a release or two just to be sure.
There was a problem hiding this comment.
Technically, if Firebase 6 pods use it, it should not be deleted until Firebase 7.
There was a problem hiding this comment.
Makes sense, that's fine with me :)
There was a problem hiding this comment.
Maybe it should be deleted since the implementation is gone. Also looks like all usages are in this repo.
There was a problem hiding this comment.
All usages are in this repo, sure, but we still call it on any SDKs that register and implement that method. (in +[FIRApp sendNotificationToSDKs:]). I think it makes sense to keep it around until next breaking change just in case folks get a new Core SDK with an older SDK that still depends on it.
| /// Implement this method if the library needs notifications for lifecycle events. This method is | ||
| /// called when the developer calls `FirebaseApp.configure()`. | ||
| + (void)configureWithApp:(FIRApp *)app; | ||
| + (void)configureWithApp:(FIRApp *)app |
There was a problem hiding this comment.
Maybe it should be deleted since the implementation is gone. Also looks like all usages are in this repo.
|
Waiting to land this until after the M52 release branch is cut. |
|
Test webhook to icore chat |
This shouldn't have been added - the component registration system can provide proper configuration instead of relying on a static `configureWithApp:` call. Many of the SDKs relied on singletons being available there, which wasn't necessary. Instead, this should happen in the component creation block and use the `instantiationTiming` parameter on `registerInternalLibrary`
This will now wait until all components have been added to the container before instantiating components. This hasn't been an issue in the past but could have caused issues if components weren't registered in the array yet.
2913811 to
053f68d
Compare
|
Rebased onto |
|
Update: this is still relevant, but FIAM needs some changes. It's a non-trivial lifecycle change, will need some planning. cc @christibbs |
The container instantiation could happen at the wrong time, before all components have been added to the container. This fixes it, and also moves IID to use the proper instantiation timing instead of relying on the `configureWithApp:` call. This is part continuation of #3147, where I'll be fixing the rest of the SDKs in follow up PRs.
* Fix container instantiation timing, IID startup. The container instantiation could happen at the wrong time, before all components have been added to the container. This fixes it, and also moves IID to use the proper instantiation timing instead of relying on the `configureWithApp:` call. This is part continuation of #3147, where I'll be fixing the rest of the SDKs in follow up PRs. * Typo * Further work on instantiation timing. This moves the instantiation of components to after the FirebaseApp is completely configured and assigned, as it should be. This also exposed a recursive issue with IID calling the Messaging singleton, which in turn called IID and instantiated multiple instances. A change was made to break the cycle: the Messaging `isAutoInitEnabled` call was changed to a static method vs an instance method, allowing IID to call it during initialization without instantiating the Messaging instance (since it doesn't have a direct dependency on it). * Revert unintentional formatting of entire FIRMessaging file. * Fix tests. * Format InstanceID.m * Remove unused import. * Fix ComponentContainer test. * Minor PR feedback.
|
Closed in favour of #4137 |
This shouldn't have been added - the component registration system can
provide proper configuration instead of relying on a static
configureWithApp:call. Many of the SDKs relied on singletons beingavailable there, which wasn't necessary. Instead, this should happen
in the component creation block and use the
instantiationTimingparameter on
registerInternalLibrary