expo-image-picker will always copy the contents of image into cache and provide a URL to cached image:
As a result we'll ALWAYS need to unlink the selection.uri. We also clean up processedMedia.uploadURI if necessary.
Details
Details
Tested small images which weren't processed and large images which were processed. Logged the contents of selection, processedMedia, and urisToBeDisposed. When I commented out the unlink line (159), I was able to open all of the images in Finder. When I uncommented the unlink line, all of the images were inaccessible because they'd been removed:
Before:
After:
Tested Android by ensuring that files remained in caches when unlink was commented out and weren't in caches when unlink was uncommented:
Selected an image avatar and ensured contents were as before:
Selected an image avatar with unlink left out and observed that new files were added:
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/avatars/avatar-hooks.js | ||
|---|---|---|
| 155–158 ↗ | (On Diff #26017) | So these unlink calls totally could be in a finally block based on what I read/verified:  However, it's probably not intuitive that the finally executes before the return in the catch block? |
Comment Actions
Please amend test plan to include testing Android as well as iOS.
Also, can we not pass filesystem.unlink in directly, without needing an extra lambda?
| native/avatars/avatar-hooks.js | ||
|---|---|---|
| 125 ↗ | (On Diff #26017) | Is it always safe to unset here? Did you test on Android? |
Comment Actions
Also, can we not pass filesystem.unlink in directly, without needing an extra lambda?
Yeah, had some temporary logging within the forEach, will pass in filesystem.unlink directly.
| native/avatars/avatar-hooks.js | ||
|---|---|---|
| 125 ↗ | (On Diff #26017) | I'm not sure I understand? |