kirillzyusko / react-native-keyboard-controller

Keyboard manager which works in identical way on both iOS and Android
https://kirillzyusko.github.io/react-native-keyboard-controller/
MIT License
1.6k stars 64 forks source link

The library conflicts with the react-native-text-input-mask since version 1.10.0. #324

Closed devoren closed 8 months ago

devoren commented 8 months ago

Describe the bug Hello! I know that this problem is not serious and in many cases does not occur. Sorry for the increase in issues, but I hope someone can help πŸ™ . The library conflicts with the react-native-text-input-mask since version 1.10.0, so the keyboard does not open the first time. I think this is related to the FocusedInputTextChangedEvent. The latest version that works without errors is 1.9.5 without FocusedInputTextChangedEvent.

Code snippet

<TextInputMask
  mask="+1 ([000]) [000] [00] [00]"
  onChangeText={(formatted, extracted) => {
    console.log(formatted); // +1 (123) 456-78-90
    setPhone(extracted ?? ''); // 1234567890
  }}
  keyboardType="phone-pad"
  placeholder="+1 (___) ___ __ __"
  style={{
    width: '100%',
    backgroundColor: '#e2e8f0',
    borderRadius: 4,
    paddingHorizontal: 12,
  }}
/>

Repo for reproducing You can change the RNKC version and see the difference. https://github.com/devoren/TestRNKC732

To Reproduce Steps to reproduce the behavior:

  1. Install react-native-keyboard-controller and react-native-text-input-mask
  2. Add Mask input and add mask prop
  3. Press input and see that the keyboard does not open the first time
  4. See logcat from android studio

Expected behavior Keyboard opens without any problem in first press

Screenshots

image

In the video i press input 2 times

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/99968085/6193afeb-82cf-4822-ab3f-0699b37826c7

Smartphone (please complete the following information):

Additional context The problem only occurs if I add a mask prop to TextInputMask.

kirillzyusko commented 8 months ago

Nice catch @devoren

I'll try to have a look today on it πŸ‘€

kirillzyusko commented 8 months ago

Just a quick update - the issue is reproducible on my end in 100% cases.

Basically this exception happens when you iterate over and collection gets modified, so this code will trigger this exception:

List<String> myList = new ArrayList<>();
Iterator<String> iterator = myList.iterator();
while (iterator.hasNext()) {
    String element = iterator.next();
    myList.add("newElement"); // This will throw ConcurrentModificationException
}

There is some issues because in different calls I see different amount of listeners.

3 listeners 2 listeners
image image

I've tried naive things like adding listener after 100ms or 1s, but it will produce crash when you type a symbol πŸ€·β€β™‚οΈ

I'll search for new solutions πŸ‘€

kirillzyusko commented 8 months ago

I think this is the most interesting issue I've ever discovered in my life πŸ‘€

So basically crash happens in this code:

public void afterTextChanged(Editable s) {
    if (!ReactEditText.this.mIsSettingTextFromJS && ReactEditText.this.mListeners != null) {
        Iterator var2 = ReactEditText.this.mListeners.iterator();

        while(var2.hasNext()) {
            TextWatcher listener = (TextWatcher)var2.next();
            listener.afterTextChanged(s);
        }
    }

}

On every afterTextChanged input-mask-android removes the listener and adds it back.

Now let's consider oversimplified example (you can run this code in https://www.jdoodle.com/online-java-compiler):

import java.util.ArrayList;
import java.util.Iterator;

public class MyClass {
    public static void main(String args[]) {
        ArrayList<Integer> mListeners = new ArrayList<>();
        mListeners.add(0);
        mListeners.add(1);
        // mListeners.add(2);

        Iterator<Integer> iterator = mListeners.iterator();

        while (iterator.hasNext()) {
            Integer listener = iterator.next();

            // Check if the listener is equal to 1
            // 1 is OnlyChangeIfRequiredMaskedTextChangedListener and we simulate the behavior of this class
            if (listener == 1) {
                int i = mListeners.indexOf(listener);

                if (i >= 0) {
                    mListeners.remove(i);
                }

                // Add the removed element at the end
                mListeners.add(listener);
            }
        }

        // Print the modified list
        System.out.println(mListeners);
    }
}

This code will be executed without any issues, and will produce expected [0, 1] output.

But if we uncomment mListeners.add(2); - we'll get our exception:

Exception in thread "main" java.util.ConcurrentModificationException
    at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1095)
    at java.base/java.util.ArrayList$Itr.next(ArrayList.java:1049)
    at MyClass.main(MyClass.java:14)

These 3 array elements are real elements in RN app:

So here we get a situation, where react-native code works for 2 listeners, but doesn't work for 3. To make this code working again with 3 listeners we just can change the order - if the order is [2, 0, 1] or [0, 2, 1] it will work, because removal/insertion operation will be the last one and java will not throw an exception.

Or before iterating we can create a copy of array and iterate over this copy (but we'll have to modify react-native source code). For example Android code iterates over mListeners as:

if (mListeners != null) {
    final ArrayList<TextWatcher> list = mListeners;
    final int count = list.size();
    for (int i = 0; i < count; i++) {
        list.get(i).afterTextChanged(text);
    }
}

Oversimplified version with new iteration approach that works may look like this:

import java.util.ArrayList;
import java.util.Iterator;

public class MyClass {
    public static void main(String args[]) {
        ArrayList<Integer> mListeners = new ArrayList<>();
        mListeners.add(0);
        mListeners.add(1);
        mListeners.add(2);

        final ArrayList<Integer> list = mListeners;
        final int count = list.size();
        for (int i = 0; i < count; i++) {
            Integer listener = list.get(i);
            if (listener == 1) {
                mListeners.remove(i);

                // Add the removed element at the end
                mListeners.add(listener);
            }
        }

        // Print the modified list
        System.out.println(mListeners);
    }
}

So now we have two options on how to fix this issue:

Both solutions are not good enough because:

I will think about other solutions too, but if you @devoren have any ideas - please, share them 😊

devoren commented 8 months ago

To be honest, I'm not good at the native side. But I understand the problem, so is it because of react-native limitations? Then I don't want to take up too much of your timeπŸ˜…. For now i will use version 1.9.5 without any problems

kirillzyusko commented 8 months ago

I think I will go with reflection approach (it should work pretty reliably) and most likely will submit a PR to react-native repo :)

I think it'll be fixed in 1.10.2 🀞

4yki commented 8 months ago

Just've tried new version. The issue was fixed for me. Huge thx for your work @kirillzyusko