iOS: Support HTTP headers for source prop on <Image> components by rigdern · Pull Request #7338 · react/react-native

rigdern · 2016-05-02T22:52:19Z

Allows developers to specify headers to include in the HTTP request
when fetching a remote image. For example, one might leverage this
when fetching an image from an endpoint that requires authentication:

<Image
style={styles.logo}
source={{
uri: 'http://facebook.github.io/react/img/logo_og.png',
headers: {
Authorization: 'someAuthToken'
}
}}
/>

Note that the header values must be strings.

Works on iOS and Android.

Test plan (required)

Ran a small example like the one above on iOS and Android and ensured the headers were sent to the server.
Ran a small example to ensure that components without headers still work.
Currently using this code in our app.

Adam Comella
Microsoft Corp.

facebook-github-bot · 2016-05-02T22:53:23Z

By analyzing the blame information on this pull request, we identified @nicklockwood and @ide to be potential reviewers.

eslint-bot · 2016-05-02T23:01:42Z

no-trailing-spaces: Trailing spaces not allowed.

ghost · 2016-05-02T23:08:15Z

@rigdern updated the pull request.

ghost · 2016-05-02T23:56:16Z

@rigdern updated the pull request.

ghost · 2016-05-04T03:07:52Z

@rigdern updated the pull request.

facebook-github-bot · 2016-05-18T20:35:21Z

@rigdern updated the pull request.

ghost · 2016-05-26T05:26:33Z

@rigdern updated the pull request.

ide · 2016-05-26T06:02:56Z

Just check if (!headers)

ide · 2016-05-26T06:15:29Z

@rigdern You're awesome for submitting both Android and iOS implementations that both look good at a glance. Would like to get more reviewers on the code, but this PR looks promising.

nicklockwood · 2016-05-26T09:41:23Z

@rigdern This is a good idea, but changing these iOS APIs will break quite a lot of our stuff internally.

I think the better approach here would be to add a new method to RCTImageLoader that takes an NSURLRequest, which we already have an RCTConvert method for processing in a standard format (it's already used by WebView.source). Validating that the headers are all strings should also be done inside RCTConvert (though it isn't currently).

The existing methods of RCTImageLoader can then be refactored to call the new method, so we don't break anything, and we can deprecate the old methods.

Also, while I echo @ide's sentiment about building both iOS and Android implementations, it's much easier for us to merge if it's split into separate PRs for each platform.

Allows developers to specify headers to include in the HTTP request when fetching a remote image. For example, one might leverage this when fetching an image from an endpoint that requires authentication: ``` <Image style={styles.logo} source={{ uri: 'http://facebook.github.io/react/img/logo_og.png', headers: { Authorization: 'someAuthToken' } }} /> ``` Note that the header values must be strings. When doing image requests, this also gives developers control over the other properties supported by [RCTConvert NSURLRequest:] like `method` and `body`. This change deprecates some APIs on RCTImageLoader in order to replace them with more flexible APIs which take an NSURLRequest rather than just an NSURL.

ghost · 2016-05-27T00:25:17Z

@rigdern updated the pull request.

rigdern · 2016-05-27T00:27:10Z

@nicklockwood As you suggested, I created several new RCTImageLoader methods which accept an NSURLRequest and deprecated the old ones which accepted an NSURL. I also removed the Android implementation of the feature and will submit it in a separate PR.

rigdern · 2016-05-27T01:40:27Z

Android PR is #7791

nicklockwood · 2016-06-01T12:13:23Z

@facebook-github-bot import

Summary: Allows developers to specify headers to include in the HTTP request when fetching a remote image. For example, one might leverage this when fetching an image from an endpoint that requires authentication: ``` <Image style={styles.logo} source={{ uri: 'http://facebook.github.io/react/img/logo_og.png', headers: { Authorization: 'someAuthToken' } }} /> ``` Note that the header values must be strings. Works on iOS and Android. **Test plan (required)** - Ran a small example like the one above on iOS and Android and ensured the headers were sent to the server. - Ran a small example to ensure that \<Image\> components without headers still work. - Currently using this code in our app. Adam Comella Microsoft Corp. Closesreact/react-native#7338 Reviewed By: javache Differential Revision: D3371458 Pulled By: nicklockwood fbshipit-source-id: cdb24fe2572c3ae3ba82c86ad383af6d85157e20

rgoldiez · 2016-07-29T13:21:59Z

@rigdern - Did this PR get accepted and, if so, which version of RN did it first appear in?

EDIT -- nevermind... I see it was accepted.

bitfused · 2016-08-31T02:14:03Z

Are there any working examples? This is not working for me and I am not sure how to debug this..

rgoldiez · 2016-08-31T02:25:14Z

@IlyaRepo You want to do something like this, which is working for me

<Image source={{
uri: url,
headers: {
'Authorization': someToken, // or whatever your header looks like
}
}}
/>

Summary: Hi there, As ospfranco found there is some documentation for the code coming from #7338 missing, so I added it. It concerns the method, headers and bdoy field on the Image source object. If you would like to have any changes made, please don't hesitate to comment, I will add them. Have a nice day and thank you for maintaining React Native! Closes#9304 Differential Revision: D4913262 Pulled By: javache fbshipit-source-id: 922430ec3388560686e1cf53cb5dff7f30e4e31f

Summary: Hi there, As ospfranco found there is some documentation for the code coming from react#7338 missing, so I added it. It concerns the method, headers and bdoy field on the Image source object. If you would like to have any changes made, please don't hesitate to comment, I will add them. Have a nice day and thank you for maintaining React Native! Closesreact#9304 Differential Revision: D4913262 Pulled By: javache fbshipit-source-id: 922430ec3388560686e1cf53cb5dff7f30e4e31f

xahon · 2017-08-31T13:39:48Z

Headers not working on ios, android works well

Summary: Allows developers to specify headers to include in the HTTP request when fetching a remote image. For example, one might leverage this when fetching an image from an endpoint that requires authentication: ``` <Image style={styles.logo} source={{ uri: 'http://facebook.github.io/react/img/logo_og.png', headers: { Authorization: 'someAuthToken' } }} /> ``` Note that the header values must be strings. Works on iOS and Android. **Test plan (required)** - Ran a small example like the one above on iOS and Android and ensured the headers were sent to the server. - Ran a small example to ensure that \<Image\> components without headers still work. - Currently using this code in our app. Adam Comella Microsoft Corp. Closesreact/react-native#7338 Reviewed By: javache Differential Revision: D3371458 Pulled By: nicklockwood fbshipit-source-id: cdb24fe2572c3ae3ba82c86ad383af6d85157e20

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels May 2, 2016

eslint-bot reviewed May 2, 2016
View reviewed changes

Comment threadLibraries/Image/Image.ios.js Outdated
eslint-botMay 2, 2016
Copy link
Copy Markdown
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.

rigdern force-pushed the rigdern/image-headers branch from 796939f to 426ad27Compare May 2, 2016 23:07

rigdern force-pushed the rigdern/image-headers branch from 426ad27 to fed4637Compare May 2, 2016 23:55

rigdern force-pushed the rigdern/image-headers branch from 1eb2054 to 32d7c77Compare May 18, 2016 20:33

rigdern force-pushed the rigdern/image-headers branch from 32d7c77 to 677ee37Compare May 26, 2016 05:18

ide reviewed May 26, 2016
View reviewed changes

Comment threadReact/Base/RCTImageSource.m Outdated
ideMay 26, 2016
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just check if (!headers)

ide added GH Review: needs-revision and removed GH Review: review-needed labels May 26, 2016

rigdern force-pushed the rigdern/image-headers branch from 677ee37 to ca1f5a7Compare May 27, 2016 00:24

ghost added GH Review: review-needed and removed GH Review: needs-revision labels May 27, 2016

rigdern changed the title ~~Support HTTP headers for source prop on <Image> components~~iOS: Support HTTP headers for source prop on <Image> componentsMay 27, 2016

rigdern mentioned this pull request May 27, 2016
Android: Support HTTP headers for source prop on <Image> components #7791
Closed

ghost closed this in ee8496fJun 1, 2016

DanielMSchmidt mentioned this pull request Aug 9, 2016
Added documentation for additional fields of Image #9304
Closed

rigdern mentioned this pull request Sep 10, 2016
Support HTTP headers for source prop on <Image> components microsoft/react-native-windows#689
Closed

This pull request was closed.

Uh oh!

Conversation

rigdern commented May 2, 2016

Uh oh!

facebook-github-bot commented May 2, 2016

Uh oh!

eslint-botMay 2, 2016

Choose a reason for hiding this comment

Uh oh!

ghost commented May 2, 2016

Uh oh!

ghost commented May 2, 2016

Uh oh!

ghost commented May 4, 2016

Uh oh!

facebook-github-bot commented May 18, 2016

Uh oh!

ghost commented May 26, 2016

Uh oh!

ideMay 26, 2016

Choose a reason for hiding this comment

Uh oh!

ide commented May 26, 2016

Uh oh!

nicklockwood commented May 26, 2016

Uh oh!

ghost commented May 27, 2016

Uh oh!

rigdern commented May 27, 2016

Uh oh!

rigdern commented May 27, 2016

Uh oh!

nicklockwood commented Jun 1, 2016

Uh oh!

rgoldiez commented Jul 29, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bitfused commented Aug 31, 2016

Uh oh!

rgoldiez commented Aug 31, 2016

Uh oh!

xahon commented Aug 31, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

rgoldiez commented Jul 29, 2016•
edited
Loading