facebookarchive / react-native-fbsdk

A React Native wrapper around the Facebook SDKs for Android and iOS. Provides access to Facebook login, sharing, graph requests, app events etc.
https://developers.facebook.com/docs/react-native
Other
2.99k stars 908 forks source link

Use locale insensitive toUpperCase and toLowerCase #729

Open kyo504 opened 4 years ago

kyo504 commented 4 years ago

Fixes # #728

Test Plan:

Before this PR, if you run following example,

export default class App extends Component<{}> {
  _login = async () => {
    try {
      LoginManager.setLoginBehavior('native_with_fallback');
      await LoginManager.logInWithPermissions(['email']);
    } catch (e) {
      Alert.alert(JSON.stringify(e));
    }
  };

  render() {
    return (
      <View style={styles.container}>
        <TouchableHighlight onPress={this._login}>
          <Text>Login</Text>
        </TouchableHighlight>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#F5FCFF',
  },
});

you could see the following error on Android Debug Console.

E/unknown:ReactNative: Exception in native call
    java.lang.IllegalArgumentException: No enum constant com.facebook.login.LoginBehavior.NATİVE_WİTH_FALLBACK
        at java.lang.Enum.valueOf(Enum.java:258)
        at com.facebook.login.LoginBehavior.valueOf(LoginBehavior.java:26)
        at com.facebook.reactnative.androidsdk.FBLoginManagerModule.setLoginBehavior(FBLoginManagerModule.java:100)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:158)
        at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
        at android.os.Looper.loop(Looper.java:193)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:232)
        at java.lang.Thread.run(Thread.java:764)

And this leads crash on release mode. I've tested that app works fine after this pr.

mifi commented 4 years ago

Maybe better to not use a case transformation for such keys at all and instead replace it with a more robust transformation logic?

kyo504 commented 4 years ago

@mifi yeah, that would be another way to solve this problem. But I don't know the history of current implementation and what you mentioned would cause backward compatibility issue(I think this is not as important as all the people should bump major version).

janicduplessis commented 4 years ago

I agree with @mifi, can we use a switch statement to return the proper enum value instead?