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

[android][image-picker] fix inconsistent naming of filesize property (filesize vs fileSize) #27293

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

WookieFPV
Copy link
Contributor

rename filesize to fileSize.
Android Implementation was different from iOS, ts definitions (ImagePicker.types.ts) & docu.
This was probably not intended and a bug

Why

The naming of filesize was different on iOS (fileSize) and Android (filesize).
This PR should unify the naming.
Support for fileSize in Android was added in #24524 but this improves the naming to make it the same on iOS & Android.

Naming:
filesize: Android
fileSize: iOS, typescript, docs

How

Replace in Files "filesize" -> "fileSize" in the whole packages/expo-image-picker/android folder.

Test Plan

I did not test this feature.
This change is fairly simple.
But consumer that rely on the "wrong" field name might get a breaking change if this bug is fixed

Checklist

…tation was different from iOS, ts definitions (ImagePicker.types.ts) & docu.
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Feb 26, 2024
Copy link
Collaborator

@alanjhughes alanjhughes left a comment

Choose a reason for hiding this comment

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

Thank you! Congrats on your first contribution! 🔥

I am not sure if this change should be marked as breaking.
It is breaking but it fixes a wrong behaviour and this makes the implementation match the docs
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Feb 26, 2024
@alanjhughes alanjhughes merged commit 35ba160 into expo:main Feb 28, 2024
7 checks passed
@MarkSFrancis
Copy link

MarkSFrancis commented Mar 6, 2024

This bug just caught us out too, thanks @WookieFPV!
We'll keep an eye on the release notes to make sure we change our code from filesize for Android to fileSize once this is released.

@nimeshmaharjan1
Copy link

bro is this still on dev? this is really causing me an issue rn

@brentvatne brentvatne added the published Changes from the PR have been published to npm label Apr 18, 2024
@Adriano2108
Copy link

Is this merged into SDK 51, i am using SDK 50 and i still have this issue

@entiendoNull
Copy link
Contributor

Fixes #30895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants