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

iOS Fix: Use RCTImageLoaderProtocol for ReactNative v0.61.0 #3136

Conversation

fenderface66
Copy link
Contributor

Does any other open PR do the same thing?

No

(please answer here)

What issue is this PR fixing?

RCTImageLoader no longer works in iOS builds for RN>0.61.0. You need to use RCTIMageLoaderProtocol instead.
(please link the issue here)

How did you test this PR?

By successfully building the project after these changes were made.

  1. Create a RN0.61.0 project
  2. Link react-native-maps
  3. Try to build iOS

(please answer here)

@rborn
Copy link
Collaborator

rborn commented Oct 16, 2019

Hi @fenderface66 thnx for the PR, doesn't #3125 already fix this issue?

@fenderface66
Copy link
Contributor Author

@rborn Unfortunately not because that PR still tries to import the ImageLoader which causes the build to fail:

image

@rborn
Copy link
Collaborator

rborn commented Oct 16, 2019

@brien-crean can you please jump in and you guys make the best of both PRs? ❤️ My ObjC is less than rusty😅 so I'd need help here 🤗

@fenderface66
Copy link
Contributor Author

@brien-crean what do you think? I'd love to get this merged soon so that I don't have to use my own fork 😄

@qyhongfan
Copy link

same issue

@brien-crean
Copy link
Contributor

@rborn @fenderface66 I just tried this out. This looks great to me, thanks @fenderface66! I say merge this on top of what was already merged from mine as it fixes the missing RCTIMageLoaderProtocol from my PR

@James-E-Adams
Copy link

James-E-Adams commented Oct 23, 2019

Looks great - This is one of the blockers for us to bump to RN0.61.x. Anything I can do to help get this out in a release? Would love to avoid vendoring react-native-maps.

Edit: As an aside, is this implementation still backwards compatible with previous RN versions?

@rborn
Copy link
Collaborator

rborn commented Oct 23, 2019

@brien-crean @fenderface66 ❤️

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.

5 participants