react-native-documents / document-picker

Document Picker for React Native
https://react-native-documents.github.io/
MIT License
1.35k stars 437 forks source link

fix(ios): decode URI for iOS #544

Closed barun1997 closed 2 years ago

barun1997 commented 2 years ago

Summary

There is a common issue related to space the file names in iOS. But, this may not be apparent when debugging.

In order to resolve it, you need to decode the URI.

@vonovak has also suggested the same solution in https://github.com/rnmods/react-native-document-picker/issues/199#issuecomment-932807046

There are other issues that mention it: https://github.com/rnmods/react-native-document-picker/issues/350, https://github.com/rnmods/react-native-document-picker/issues/291, and https://github.com/rnmods/react-native-document-picker/issues/13

What's required for testing (prerequisites)?

Compatibility

OS Implemented
iOS
Android

Checklist

vonovak commented 2 years ago

Hello and thanks for the PR! I'm not in favor of changing the current behavior in the proposed way: the module returns a uri, and uris use the percent encoding. If we decode the uri on the module side, it will no longer make sense for the field to be called uri. This change is also a breaking change for all those who did or did not (based on their use case) decode the uri returned from the module. We could add another field that would be called eg. path and that would work on iOS but it probably not make sense on Android because the uri returned there is a ContentResolver uri and not a file path like in iOS. Not even sure how about windows.

To sum it up, I agree this might be pain point but the solution I'd prefer is to highlight this in the readme and make sure consumers double check whether or not they want to decode the uri. If you change the PR so that it improves the readme, I'll be happy to merge! ;)

Thank you! 🙂

barun1997 commented 2 years ago

@vonovak Thank you for the review and elaborate explanation.

I agree - that an alternative solution here would be to improve the README so people can check whether they want to decode the uri.