Trashcoder/blob slice content type #38078
Conversation
facebook-github-bot
added
the
CLA Signed
Base commit: 71f715a |
|
Is there something i can do to make the PR pass? It seems these 3 fails were there because of a server error. |
|
Congrats on your 1st PR to RN 🎉
Those seem transient issues, unrelated to changes in this PR so no need to worry about them. |
done. |
Co-authored-by: Pranav Yadav <[email protected]>
|
|
||
| blob.data.size = 34546; | ||
|
|
||
| const slice = blob.slice(0, 2354, 'text/plain'); |
There was a problem hiding this comment.
| const slice = blob.slice(0, 2354, 'text/plain'); | |
| const slice = blob.slice(0 /* start */, 2354 /* end */, 'text/plain' /* contentType */); |
Not necessary to do this, still it's better to make tests more readable like this or some other way
There was a problem hiding this comment.
Left format suggestions and tips.
Also, something important this PR doesn't add is "negative tests".
We should always add the tests for negative cases e.g. malformed & empty contentType
P.S.: This practice has helped me avoid potential regressions 😅
Overall, this is awesome ❤️
| @@ -77,6 +77,18 @@ describe('Blob', function () { | |||
| expect(sliceC.data.offset).toBe(34543); | |||
| expect(sliceC.size).toBe(Math.min(blob.data.size, 34569) - 34543); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
@trashcoder You should always run yarn lint && yarn prettier before commiting 🚀 :)
In this PR only. It's part of the development Workflow you can check the the guide here: https://reactnative.dev/contributing/how-to-contribute-code#development-workflow For the scope of this PR below commands should suffice:
Let me know if you need any further help. |
|
LGTM, fix the formatting and we should get this in. |
|
@blakef formatting is already fixed. |
|
@blakef has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
|
Summary: For the contentType Parameter of the method slice in Class Blob which was fixed for #38078 the typescript declaration is missing. ## Changelog: [GENERAL] [ADDED] - Added contentType parameter to Blob declaration <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #38163 Test Plan: I ran yarn run test-typescript and check with Webstorm and VSCode. Reviewed By: rshest Differential Revision: D47228992 Pulled By: javache fbshipit-source-id: fd767bb47c5e64657bfafba4c84d1d8671857109
Summary:
I added the contentType parameter to Blob.slice like it's in the MDN Web docs.
This PR fixes #38058
When i slice a Blob for chunked uploads with react native i lost the content type, e.g. "image/jpeg", so the server doesn't know what kind of file he gets. In the docs of MDN the slice method was described with a third contentType parameter which was missing in Metas implementation.
Changelog:
[GENERAL] [ADDED] added a third parameter "contentType" to method slice of class Blob.
Test Plan:
I tested it with the unit-tests:
yarn run test Blob-test.js
yarn run v1.22.19
$ jest Blob-test.js
PASS packages/react-native/Libraries/Blob/tests/Blob-test.js
Blob
✓ should create empty blob (5 ms)
✓ should create blob from other blobs and strings
✓ should slice a blob (1 ms)
✓ should slice a blob and sets a contentType
✓ should close a blob (4 ms)
My added unit test results "✓ should slice a blob and sets a contentType".