Skip to content

fix(iOS): unify prefetchImageWithMetadata's signature in JS and ObjC land#47532

Closed
TheRogue76 wants to merge 2 commits intofacebook:mainfrom
TheRogue76:main
Closed

fix(iOS): unify prefetchImageWithMetadata's signature in JS and ObjC land#47532
TheRogue76 wants to merge 2 commits intofacebook:mainfrom
TheRogue76:main

Conversation

@TheRogue76
Copy link
Contributor

@TheRogue76 TheRogue76 commented Nov 8, 2024

Summary:

in prefetchImageWithMetadata's implementation in ObjC, the method's queryRootName is treated as being nullable. The image spec for it in JS (and the Codegened ObjC header that gets built on top of it) treat the field as not nullable. This change makes the field nullable in the spec to match up what we have in the implementation.

I also noticed that the method is not defined in the Image props on the RN website, so perhaps we should add this there as well.

Changelog:

[IOS] [CHANGED] - make prefetchImageWithMetadata's queryRootName nullable in the spec

Test Plan:

yarn test:
Screenshot 2024-11-09 at 00 36 30
It has no iOS specific tests, so nothing to run on that side

@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 Nov 8, 2024
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

thanks for this fix!

@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.

@TheRogue76
Copy link
Contributor Author

No idea how i managed to kill the poor CI for it to take 165 minutes. @cipolleschi Did it say what had gone wrong?

cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 18, 2024
…nal (facebook#47532)

Summary:
in `prefetchImageWithMetadata`'s implementation in ObjC, the method's `queryRootName` is treated as being nullable. The image spec for it in JS (and the Codegened ObjC header that gets built on top of it) treat the field as not nullable. This change makes the field nullable in the spec to match up what we have in the implementation.

I also noticed that the method is not defined in the [Image props](https://reactnative.dev/docs/image) on the RN website, so perhaps we should add this there as well.

## Changelog:

[IOS] [CHANGED] - make `prefetchImageWithMetadata`'s `queryRootName` nullable in the spec

Pull Request resolved: facebook#47532

Test Plan:
yarn test:
<img width="1576" alt="Screenshot 2024-11-09 at 00 36 30" src="https://github.com/user-attachments/assets/4162ff79-1388-4f6f-9576-256fd9011fcf">
It has no iOS specific tests, so nothing to run on that side

Differential Revision: D65761208
cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 18, 2024
…land (facebook#47532)

Summary:
in `prefetchImageWithMetadata`'s implementation in ObjC, the method's `queryRootName` is treated as being nullable. The image spec for it in JS (and the Codegened ObjC header that gets built on top of it) treat the field as not nullable. This change makes the field nullable in the spec to match up what we have in the implementation.

I also noticed that the method is not defined in the [Image props](https://reactnative.dev/docs/image) on the RN website, so perhaps we should add this there as well.

## Changelog:

[IOS] [CHANGED] - make `prefetchImageWithMetadata`'s `queryRootName` nullable in the spec

Pull Request resolved: facebook#47532

Test Plan:
yarn test:
<img width="1576" alt="Screenshot 2024-11-09 at 00 36 30" src="https://github.com/user-attachments/assets/4162ff79-1388-4f6f-9576-256fd9011fcf">
It has no iOS specific tests, so nothing to run on that side

Differential Revision: D66093745

Pulled By: cipolleschi
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 18, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 4dd60ac.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @TheRogue76 in 4dd60ac

When will my fix make it into a release? | How to file a pick request?

rezkiy37 added a commit to rezkiy37/react-native that referenced this pull request Nov 18, 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.

4 participants