Skip to content

fix: images posted to server via fetch get their size inflated significantly for iOS #6749

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

Merged
merged 2 commits into from
May 16, 2022

Conversation

dimatretyak
Copy link
Contributor

@dimatretyak dimatretyak commented May 16, 2022

Related PR #6743

Description

While testing the beta build, I noticed a strange behavior on iOS: when I tried to upload a file, I received an error saying that the maximum allowed file size was exceeded (although the same file was uploaded on Android without any problems). It turned out that the problem has been around for a long time in React Native and and you can find a discussion of it here.

Solution based on this comment

Demo

Before

before.mp4

After

after.mp4

PR Checklist

  • I tested my changes on iOS / Android.
  • I added screenshots or videos to illustrate my changes.
  • I added Tests and Stories for my changes.
  • I added an app state migration.
  • I hid my changes behind a feature flag.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

  • Fixed a problem with increasing file size for iOS - dimatretyak

Need help with something? Have a look at our docs, or get in touch with us.

@dimatretyak dimatretyak self-assigned this May 16, 2022
@dimatretyak dimatretyak force-pushed the dimatretyak/fix/resized-image-size-for-ios branch from fc979fd to c6079fe Compare May 16, 2022 12:45
@dimatretyak dimatretyak requested review from dblandin and gkartalis May 16, 2022 12:53
@dimatretyak dimatretyak marked this pull request as ready for review May 16, 2022 12:53
@dimatretyak dimatretyak changed the title fix: file size for iOS fix: image posted to server via fetch get their size inflated significantly for iOS May 16, 2022
@dimatretyak dimatretyak changed the title fix: image posted to server via fetch get their size inflated significantly for iOS fix: images posted to server via fetch get their size inflated significantly for iOS May 16, 2022
gkartalis
gkartalis previously approved these changes May 16, 2022
dblandin
dblandin previously approved these changes May 16, 2022
Copy link
Member

@dblandin dblandin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird bug! Nice work hunting down a workaround. 💯

Should we also document this hack in our HACKS.md file?

@dimatretyak
Copy link
Contributor Author

Should we also document this hack in our HACKS.md file?

Yes, I think it would not be superfluous to do this

@dimatretyak dimatretyak dismissed stale reviews from dblandin and gkartalis via d87944a May 16, 2022 15:26

#### When we can remove this:

When this problem will be fixed in one of the next React Native releases (fixes were already added in 0.66.4, but were eventually [reversed](https://github.com/facebook/react-native/commit/e83feffeba567b4e304181632128ee1caa036c45))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: This is great context to capture. Thank you!! 👏🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@dimatretyak dimatretyak merged commit f071496 into main May 16, 2022
@dimatretyak dimatretyak deleted the dimatretyak/fix/resized-image-size-for-ios branch May 16, 2022 16:23
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 this pull request may close these issues.

3 participants