Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.59k stars 2.92k forks source link

Import & Export Onyx state reliability issues #53060

Open TMisiukiewicz opened 1 week ago

TMisiukiewicz commented 1 week ago

Exporting and importing Onyx state features are highly valuable in scenarios such as:

However, there is significant room for improvement in both features to maximize their potential and ensure reliability. The current issues include the following:

Export

Import

All these issues collectively affect the reliability of the export and import features, reducing their effectiveness for debugging and performance optimization tasks. Addressing these problems is crucial for ensuring their value in real-world scenarios.

TMisiukiewicz commented 5 days ago

While recording videos with a fix for proper displaying LHN elements I noticed the Import is broken on iOS. I'll focus on fixing this one first before I open any other PR since it blocks testing on the platform entirely image

TMisiukiewicz commented 3 days ago

Found a solution for above error. The react-native-fs library is not maintained anymore for more than 2 years now, and it is expecting NSInteger pointer for both position and length parameters of read function. According to the React Native docs, using NSInteger should be avoided.

So we have a couple of options:

  1. Patch the library and switch from using NSInteger to NSNumber
  2. Replace the library with it's fork, https://github.com/birdofpreyru/react-native-fs, which is actively maintained and supports new architecture
  3. Use react-native-blob-util filesystem that we already have in the codebase. It supports reading files as stream, so i think it should work as well.

From my perspective, the fastest way to unblock us with improvements of the Import feature is using a patch. However the last one sounds reasonable as well. Which one should we follow? We can always go with a patch first as it's a quick one (and I already have it working) and rewrite it to readStream in the future

mountiny commented 3 days ago

If we already use react-native-blob-util in the app and it can be used here too without issues and its maintained, then lets use that instead of patching the unmaintained library.