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

feature: Update Image.getSize/getSizeWithHeaders methods to return a promise #42895

Closed

Conversation

retyui
Copy link
Contributor

@retyui retyui commented Feb 6, 2024

Summary:

Image.getSize/getSizeWithHeaders are still working in old fashioned "callback" way

Image.getSize(uri, function success(width,height) {  }, function failure(){   } ); // undefined
Image.getSizeWithHeaders(uri, headers, function success(width,height) {  }, function failure(){   } ); // undefined

But in 2024 more developers prefer use async/await syntax for asynchronous operations

So, in this PR I added support for Promise API with backward compatibility, modern way:

Image.getSize(uri).then(({width,height}) => {    }); // Promise
Image.getSizeWithHeaders(uri, headers).then(({width,height}) => {    }); // Promise

Changelog:

[GENERAL] [ADDED] - Image.getSize/getSizeWithHeaders method returns a promise if you don't pass a success callback

Test Plan:

  1. ts: New test cases added in typescript tests
  2. runtime: you can create a new project and put code from this PR into the next files
    a. node_modules/react-native/Libraries/Image/Image.android.js
    b. node_modules/react-native/Libraries/Image/Image.ios.js

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 6, 2024
@retyui retyui changed the title feature: Update Image.getSize method to return a promise feature: Update Image.getSize/getSizeWithHeaders methods to return a promise Feb 6, 2024
@retyui retyui force-pushed the feature/retyui/get-size-promise-like-api branch 2 times, most recently from e54c366 to c690ad9 Compare February 6, 2024 22:07
@retyui retyui requested a review from javache February 6, 2024 22:08
@retyui

This comment was marked as outdated.

@cipolleschi
Copy link
Contributor

/rebase - this commet will rebase the PR on top of main automatically.

All these changes are JS changes only, there is no reason why the native builds should fail. Let's see if a rebase fixes CI.

@github-actions github-actions bot force-pushed the feature/retyui/get-size-promise-like-api branch from c690ad9 to 65fb75c Compare February 7, 2024 14:16
@retyui retyui force-pushed the feature/retyui/get-size-promise-like-api branch from 65fb75c to 46bb41f Compare February 7, 2024 15:43
@retyui retyui force-pushed the feature/retyui/get-size-promise-like-api branch from 46bb41f to 42bd4aa Compare February 7, 2024 15:44
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,233,283 -187
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,600,121 +53
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: c1b8f37
Branch: main

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@retyui
Copy link
Contributor Author

retyui commented Feb 26, 2024

the Facebook Internal - Builds & Tests job has failed, do I need to change smth? @cipolleschi

@cipolleschi
Copy link
Contributor

I was looking into that. There is an internal E2E test that is failing and I need to understand why. I'll probably look into it tomorrow morning, as the day is almost over and I have another meeting coming up. :(
(it's the only test that is failing, all the other signals are green... ¯_(ツ)_/¯ )

Copy link

This pull request was successfully merged by @retyui in 2c1bcba.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Feb 27, 2024
@retyui retyui deleted the feature/retyui/get-size-promise-like-api branch February 27, 2024 10:54
retyui added a commit to retyui/react-native-website that referenced this pull request Mar 1, 2024
retyui added a commit to retyui/react-native-website that referenced this pull request Mar 1, 2024
Abbondanzo pushed a commit to facebook/react-native-website that referenced this pull request Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants