Repository navigation
feat(ios): support the fileName option for tmpfile captures (#552) - #760
Open
officialasishkumar wants to merge 1 commit into
Open
officialasishkumar wants to merge 1 commit into
officialasishkumar wants to merge 1 commit into
Conversation
`fileName` was documented as Android only: iOS ignored it and always wrote `tmp/ReactNative/<UUID>.<ext>`, so apps sharing a capture (e.g. through react-native-share) could not control the name the receiving app shows. iOS now gives `fileName` the same meaning as on Android, where it is the prefix passed to File.createTempFile: the file is `tmp/ReactNative/<fileName><UUID>.<ext>`. Keeping the unique suffix means two captures with the same fileName never overwrite each other, and every capture stays a direct child of tmp/ReactNative, the only directory releaseCapture deletes from. Like Android, a fileName shorter than 3 characters rejects the capture; a fileName containing '/' or a NUL byte is rejected too, so it cannot leave that directory. The path logic lives in a private class method so the XCTest target can cover it without an RCTBridge: default name unchanged, prefix and uniqueness with a fileName, and rejection of invalid names. README and the CaptureOptions JSDoc now describe the option for both platforms.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the iOS filename-prefix support change.
No open issues 🎉
What changed in this PR
Adds iOS support for the fileName capture option, addressing #552 with Android-compatible prefix semantics while preserving unique temporary filenames and existing cleanup behavior.
Changes:
- Adds an iOS temporary-path helper that validates prefixes and retains the unique suffix generated by
RCTTempFilePath. - Adds XCTest coverage for default filenames, unique prefixed filenames, writable paths, and invalid input.
- Documents
fileNamesupport and prefix behavior for both Android and iOS.
| File | Description |
|---|---|
src/index.tsx |
Updates CaptureOptions.fileName documentation for both platforms. |
README.md |
Describes filename prefixes, uniqueness, and validation constraints. |
ios/RNViewShot.mm |
Implements validated filename prefixes for temporary captures. |
example/ios/ViewShotExampleTests/RNViewShotEncodingTests.mm |
Tests default, prefixed, and invalid temporary filenames. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #552
What: iOS now reads the
fileNamecapture option. Whenresultis"tmpfile", the file is written totmp/ReactNative/<fileName><UUID>.<ext>instead of alwaystmp/ReactNative/<UUID>.<ext>. WithoutfileName, nothing changes.Why:
fileNamewas Android only, so on iOS the name a share target shows (WhatsApp, Mail, etc.) could not be controlled. I gave it the same meaning as on Android, where it is the prefix passed toFile.createTempFile(as #740 noted), instead of the exact-name approach from the patch in the issue:fileNamenever overwrite each other, andreleaseCaptureon one cannot delete the other;tmp/ReactNative, the only directoryreleaseCapturedeletes from, so releasing these captures still works;RCTTempFilePath, so its first-use cleanup oftmp/ReactNativekeeps working the same way.Like Android, a
fileNameshorter than 3 characters rejects the capture. AfileNamecontaining/or a NUL byte is rejected too (fileName must be at least 3 characters long and must not contain '/'), so it cannot point outside the directory.How: The path logic is a private class method,
+[RNViewShot tempFilePathWithExtension:fileName:error:], which the tmpfile branch calls instead ofRCTTempFilePath. The XCTest target can call it without anRCTBridge, in the same wayRNViewShotReleaseCaptureTestscallsreleaseCapture:. The README and theCaptureOptionsJSDoc now describefileNamefor both platforms.Tests: New cases in
RNViewShotEncodingTests.mm:fileName, the result is still a bare<UUID>.pngin theRCTTempFilePathdirectory;fileName, the name starts with the prefix, ends with the extension, stays in that same directory, differs between two calls, and can be written;"","ab","../escape","dir/name"and a name containing NUL are rejected with an error.Verified:
npm run format:check,npm run lint:ci,npm run type-checkandnpm test(104 tests) pass.RCTTempFilePath/RCTErrorWithMessagestubbed to match React Native) and ran the same checks the XCTests make; they all passed. TheiosCI job runs the real XCTest target.