[Android] [Fixed] - Fix InputAccessoryView crash on Android

[hduprat]

Summary

InputAccessoryView works fine on iOS, but crashes on Android - you can see that by using an Android device on the Expo Snack from the official doc.
It forces the developer not to render the component on Android, which is usually good, but other components have implemented other, safer ways to deal with incompatibility issues.

I am of course open to discussion about this change, as well as other implementation ideas.

Changelog

[Android] [Fixed] - Fix InputAccessoryView crash on Android

Test Plan

yarn test gives out the following output:

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,824,160	-7
android	hermes	armeabi-v7a	7,211,020	-13
android	hermes	x86	8,134,713	-7
android	hermes	x86_64	8,114,961	+0
android	jsc	arm64-v8a	9,690,753	+17
android	jsc	armeabi-v7a	8,446,850	+17
android	jsc	x86	9,642,144	+17
android	jsc	x86_64	10,239,313	+19

Base commit: f97c6a5
Branch: main

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 8dded42
Branch: main

[cortinico]

Thanks for sending this over @hduprat

I am of course open to discussion about this change, as well as other implementation ideas.

I'd say we should rather rename the file to be InputAccessoryView.ios.js and create InputAccessoryView.android.js by just exporting Unimplemented view (as we do for other components like https://github.com/facebook/react-native/tree/main/Libraries/Components/DrawerAndroid)

[hduprat]

I'd say we should rather rename the file to be InputAccessoryView.ios.js and create InputAccessoryView.android.js by just exporting Unimplemented view (as we do for other components like https://github.com/facebook/react-native/tree/main/Libraries/Components/DrawerAndroid)

Thank you for your suggestion @cortinico , I implemented it.
However there is a failing test and I do not know why.

[cortinico]

However there is a failing test and I do not know why.

Please rebase and it should be green

[hduprat]

Please rebase and it should be green

There's still an error, but I see the CI fails on the main branch too.

[hduprat]

@cortinico I have just rebased and all tests have finally passed. Thank you for fixing the Android tests 🙏

[facebook-github-bot]

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[cortinico]

From an internal discussion:

Since InputAccessoryView is kind of like a "portal" (i.e. components render output is rendered inside a special native container), I think it would be better to make this return null.

Also, we wouldn't require call sites to use Flow suppressions or hacks.
If we wanted to do this a bit more efficiently (with dead code elimination), we could do something like:

function InputAccessoryView(props: Props): React.Node {
  if (Platform.OS === 'ios') {
    // Lines 95 - 106.
  } else {
    console.warn('…');
    return null;
  }
}
module.exports = InputAccessoryView;

cc @yungsters

[hduprat]

I updated my PR according to the comment.

However, it does not cover the case where InputAccessoryView wraps a TextInput - it would display nothing instead of the TextInput itself. Wouldn't it be considered as a bug later? Or do we want to be clear with developers that InputAccessoryView should never be used on Android?

Either way, it is better than a crash.

[facebook-github-bot]

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @hduprat in afa5df1.

^{When will my fix make it into a release? | Upcoming Releases}