Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fileCopyUri has file:// prefix on iOS but doesn't on Android #526

Closed
kidroca opened this issue Jan 17, 2022 · 4 comments · Fixed by #527
Closed

fileCopyUri has file:// prefix on iOS but doesn't on Android #526

kidroca opened this issue Jan 17, 2022 · 4 comments · Fixed by #527

Comments

@kidroca
Copy link
Contributor

kidroca commented Jan 17, 2022

Bug report

Summary

Using RN document picker to select a document with the copyTo option doesn't return the URI scheme on Android while it does for iOS

If you try to use the fileCopyUri to upload a file (using vanilla fetch) it works on iOS, but does not work on Android

Adding the file:// prefix, fixes the issue on android

Reproducible sample code

import RNDocumentPicker from 'react-native-document-picker';

const documentPickerOptions = {
    type: [RNDocumentPicker.types.allFiles],
    copyTo: 'cachesDirectory',
};

RNDocumentPicker.pick(documentPickerOptions)
  .then(documents => documents.map(({ fileCopyUri, ...doc })=> {
    const payload = {
        ...doc,
    };

    // When the URI is lacking uri scheme - file upload would fail
    const hasScheme = /^.+:\/\//.test(fileResult.uri);
    if (!hasScheme) {
        payload.uri = `file://${fileCopyUri}`;
    }

   return payload;
  });

Steps to reproduce

  1. Use Document Picker with copyTo: 'cachesDirectory' option
  2. Inspect fileCopyUri under Android - no file:/// prefix
  3. Inpsect fileCopyUri under iOS - has file:/// prefix

Describe what you expected to happen:

  1. Use Document Picker with copyTo: 'cachesDirectory' option
  2. fileCopyUri should behave the same way on all Platforms
    • preferable all should have file:/// prefix
    • alternatively none should have it

Environment info

npx react-native info output:

System:
    OS: Windows 10 10.0.19044
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 20.20 GB / 31.93 GB
  Binaries:
    Node: 14.18.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.15 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.14.15 - C:\Program Files\nodejs\npm.CMD
    Watchman: 20210110.135312.0 - D:\watchman-v2021.01.11.00-windows\bin\watchman.EXE
  SDKs:
    Android SDK:
      API Levels: 27, 28, 29, 30, 31
      Build Tools: 29.0.2, 30.0.2, 31.0.0
      System Images: android-29 | Google APIs Intel x86 Atom
      Android NDK: Not Found
    Windows SDK: Not Found
  IDEs:
    Android Studio: Not Found
    Visual Studio: 16.9.31025.194 (Visual Studio Community 2019)
  Languages:
    Java: Not Found
  npmPackages:
    @react-native-community/cli: 6.2.0 => 6.2.0 
    react: ^17.0.2 => 17.0.2 
    react-native: 0.66.4 => 0.66.4 
    react-native-windows: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

library version: 7.1.3

iOS / Android version: ios 15 / Android 10

@kidroca
Copy link
Contributor Author

kidroca commented Jan 17, 2022

@vonovak
Copy link
Collaborator

vonovak commented Jan 17, 2022

Hello,
thanks for reporting.

preferable all should have file:/// prefix

this sounds like the right solution, wdyt? Feel free to send over a PR. Having some before + after screenshots will likely speed up the review as I won't have to spend so much time testing and will be nice as documentation for everyone. Thank you :)

@kidroca
Copy link
Contributor Author

kidroca commented Jan 17, 2022

preferable all should have file:/// prefix

this sounds like the right solution, wdyt?

I agree
From what I've seen having file:/// works for image srcs and http calls
Not having it still works for images but doesn't work for http calls

Having some before + after screenshots will likely speed up the review as I won't have to spend so much time testing and will be nice as documentation for everyone. Thank you :)

Alright, I'll prepare a PR tomorrow and include shots of fileCopyUri before and after the change

@kidroca
Copy link
Contributor Author

kidroca commented Jan 18, 2022

PR ready: #527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants