Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Remove categories#1226

Merged
cyanglaz merged 14 commits intoflutter:masterfrom
cyanglaz:remove_categories
Feb 15, 2019
Merged

Remove categories#1226
cyanglaz merged 14 commits intoflutter:masterfrom
cyanglaz:remove_categories

Conversation

@cyanglaz
Copy link
Contributor

Fixing flutter/flutter#27361 and flutter/flutter#27960

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera

Copy link
Contributor

@mehmetf mehmetf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why categories is a no-no? How do we prevent them from popping up again.

The PR itself LGTM as it is just an implementation detail that does not leak to Dart API.

@cyanglaz
Copy link
Contributor Author

Is there a reason why categories is a no-no? How do we prevent them from popping up again.

The PR itself LGTM as it is just an implementation detail that does not leak to Dart API.

If there are multiple categories methods that have the same name in different plugins, and an APP imported all those plugins, when in run time, objective-c will pick up a random one and use it for all of the invocations. So it can break the project since the implementations of these same named category could be different. Details in here flutter/flutter#27960.
I don't know how to restrict the category usage, maybe a custom code scanning script in CI?

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cyanglaz cyanglaz merged commit 0a923c1 into flutter:master Feb 15, 2019
@cyanglaz cyanglaz deleted the remove_categories branch February 15, 2019 22:54
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
Fixing [flutter/flutter#27361](flutter/flutter#27361) and [flutter/flutter#27960](flutter/flutter#27960)

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
Fixing [flutter/flutter#27361](flutter/flutter#27361) and [flutter/flutter#27960](flutter/flutter#27960)

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
Fixing [flutter/flutter#27361](flutter/flutter#27361) and [flutter/flutter#27960](flutter/flutter#27960)

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera
cyanglaz pushed a commit that referenced this pull request Feb 25, 2019
… nil on getFlutterError (#1274)

When `getFlutterError` was introduced(#1226), some plugins started to throw exceptions for successful operations. This fixed the issue. Related issues include flutter/flutter#28344; flutter/flutter#28065; flutter/flutter#28103
It also fixed an unintentional typo introduced in the same PR.(#1226)
romaluca pushed a commit to romaluca/plugins that referenced this pull request Mar 6, 2019
… nil on getFlutterError (flutter#1274)

When `getFlutterError` was introduced(flutter#1226), some plugins started to throw exceptions for successful operations. This fixed the issue. Related issues include flutter/flutter#28344; flutter/flutter#28065; flutter/flutter#28103
It also fixed an unintentional typo introduced in the same PR.(flutter#1226)
collinjackson pushed a commit to collinjackson/flutterfire-old2 that referenced this pull request Jun 24, 2019
… nil on getFlutterError (#1274)

When `getFlutterError` was introduced(flutter/plugins#1226), some plugins started to throw exceptions for successful operations. This fixed the issue. Related issues include flutter/flutter#28344; flutter/flutter#28065; flutter/flutter#28103
It also fixed an unintentional typo introduced in the same PR.(flutter/plugins#1226)
collinjackson pushed a commit to firebase/flutterfire that referenced this pull request Aug 14, 2019
… nil on getFlutterError (#1274)

When `getFlutterError` was introduced(flutter/plugins#1226), some plugins started to throw exceptions for successful operations. This fixed the issue. Related issues include flutter/flutter#28344; flutter/flutter#28065; flutter/flutter#28103
It also fixed an unintentional typo introduced in the same PR.(flutter/plugins#1226)
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
Fixing [flutter/flutter#27361](flutter/flutter#27361) and [flutter/flutter#27960](flutter/flutter#27960)

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera
Akachu pushed a commit to Akachu/flutter_camera that referenced this pull request Apr 27, 2020
Fixing [flutter/flutter#27361](flutter/flutter#27361) and [flutter/flutter#27960](flutter/flutter#27960)

Used static c function instead of category.
The plugins effected are
cloud_firestore
firebase_admob
firebase_auth
firebase_core
firebase_database
firebase_dynamiclink
firebase_mlvision
firebase_storage
google_signin
camera
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants