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

Jump to bottom

Allow prefetch remote image #4420

Closed
wants to merge 1 commit into from
Closed

Allow prefetch remote image #4420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 18 additions & 3 deletions Examples/UIExplorer/ImageExample.js
View file
Open in desktop
Expand Up @@ -27,11 +27,14 @@ var {
var base64Icon = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEsAAABLCAQAAACSR7JhAAADtUlEQVR4Ac3YA2Bj6QLH0XPT1Fzbtm29tW3btm3bfLZtv7e2ObZnms7d8Uw098tuetPzrxv8wiISrtVudrG2JXQZ4VOv+qUfmqCGGl1mqLhoA52oZlb0mrjsnhKpgeUNEs91Z0pd1kvihA3ULGVHiQO2narKSHKkEMulm9VgUyE60s1aWoMQUbpZOWE+kaqs4eLEjdIlZTcFZB0ndc1+lhB1lZrIuk5P2aib1NBpZaL+JaOGIt0ls47SKzLC7CqrlGF6RZ09HGoNy1lYl2aRSWL5GuzqWU1KafRdoRp0iOQEiDzgZPnG6DbldcomadViflnl/cL93tOoVbsOLVM2jylvdWjXolWX1hmfZbGR/wjypDjFLSZIRov09BgYmtUqPQPlQrPapecLgTIy0jMgPKtTeob2zWtrGH3xvjUkPCtNg/tm1rjwrMa+mdUkPd3hWbH0jArPGiU9ufCsNNWFZ40wpwn+62/66R2RUtoso1OB34tnLOcy7YB1fUdc9e0q3yru8PGM773vXsuZ5YIZX+5xmHwHGVvlrGPN6ZSiP1smOsMMde40wKv2VmwPPVXNut4sVpUreZiLBHi0qln/VQeI/LTMYXpsJtFiclUN+5HVZazim+Ky+7sAvxWnvjXrJFneVtLWLyPJu9K3cXLWeOlbMTlrIelbMDlrLenrjEQOtIF+fuI9xRp9ZBFp6+b6WT8RrxEpdK64BuvHgDk+vUy+b5hYk6zfyfs051gRoNO1usU12WWRWL73/MMEy9pMi9qIrR4ZpV16Rrvduxazmy1FSvuFXRkqTnE7m2kdb5U8xGjLw/spRr1uTov4uOgQE+0N/DvFrG/Jt7i/FzwxbA9kDanhf2w+t4V97G8lrT7wc08aA2QNUkuTfW/KimT01wdlfK4yEw030VfT0RtZbzjeMprNq8m8tnSTASrTLti64oBNdpmMQm0eEwvfPwRbUBywG5TzjPCsdwk3IeAXjQblLCoXnDVeoAz6SfJNk5TTzytCNZk/POtTSV40NwOFWzw86wNJRpubpXsn60NJFlHeqlYRbslqZm2jnEZ3qcSKgm0kTli3zZVS7y/iivZTweYXJ26Y+RTbV1zh3hYkgyFGSTKPfRVbRqWWVReaxYeSLarYv1Qqsmh1s95S7G+eEWK0f3jYKTbV6bOwepjfhtafsvUsqrQvrGC8YhmnO9cSCk3yuY984F1vesdHYhWJ5FvASlacshUsajFt2mUM9pqzvKGcyNJW0arTKN1GGGzQlH0tXwLDgQTurS8eIQAAAABJRU5ErkJggg==';

var ImageCapInsetsExample = require('./ImageCapInsetsExample');
const IMAGE_PREFETCH_URL = 'http://facebook.github.io/origami/public/images/blog-hero.jpg?r=1&t=' + Date.now();
Image.prefetch(IMAGE_PREFETCH_URL);

var NetworkImageCallbackExample = React.createClass({
getInitialState: function() {
return {
events: [],
startLoadPrefetched: false,
mountTime: new Date(),
};
},
Expand All @@ -50,9 +53,20 @@ var NetworkImageCallbackExample = React.createClass({
style={[styles.base, {overflow: 'visible'}]}
onLoadStart={() => this._loadEventFired(`✔ onLoadStart (+${new Date() - mountTime}ms)`)}
onLoad={() => this._loadEventFired(`✔ onLoad (+${new Date() - mountTime}ms)`)}
onLoadEnd={() => this._loadEventFired(`✔ onLoadEnd (+${new Date() - mountTime}ms)`)}
onLoadEnd={() => {
this._loadEventFired(`✔ onLoadEnd (+${new Date() - mountTime}ms)`);
this.setState({startLoadPrefetched: true});
}}
/>

{this.state.startLoadPrefetched ?
<Image
source={this.props.prefetchedSource}
style={[styles.base, {overflow: 'visible'}]}
onLoadStart={() => this._loadEventFired(`✔ (prefetched)onLoadStart (+${new Date() - mountTime}ms)`)}
onLoad={() => this._loadEventFired(`✔ (prefetched)onLoad (+${new Date() - mountTime}ms)`)}
onLoadEnd={() => this._loadEventFired(`✔ (prefetched)onLoadEnd (+${new Date() - mountTime}ms)`)}
/>
: null}
Copy link

Choose a reason for hiding this comment

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

You can use the && operator to render the Image component only if this.state.startLoadPrefetched is true.

<Text style={{marginTop: 20}}>
{this.state.events.join('\n')}
</Text>
Expand Down Expand Up @@ -135,7 +149,8 @@ exports.examples = [
title: 'Image Loading Events',
render: function() {
return (
Copy link

Choose a reason for hiding this comment

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

space-infix-ops: Infix operators must be spaced.

<NetworkImageCallbackExample source={{uri: 'http://facebook.github.io/origami/public/images/blog-hero.jpg?r=1'}}/>
<NetworkImageCallbackExample source={{uri: 'http://facebook.github.io/origami/public/images/blog-hero.jpg?r=1&t=' + Date.now()}}
prefetchedSource={{uri: IMAGE_PREFETCH_URL}}/>
);
},
},
Expand Down
5 changes: 5 additions & 0 deletions Libraries/Image/Image.android.js
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ var invariant = require('invariant');
var merge = require('merge');
var requireNativeComponent = require('requireNativeComponent');
var resolveAssetSource = require('resolveAssetSource');
var Networking = NativeModules.Networking;

/**
* <Image> - A react component for displaying different types of images,
Expand Down Expand Up @@ -110,6 +111,10 @@ var Image = React.createClass({

statics: {
resizeMode: ImageResizeMode,
/**
* Prefetch image for later use. Download remote image to the disk cache.
*/
prefetch: (url) => { Networking .prefetchImage(url); },
},

mixins: [NativeMethodsMixin],
Expand Down
5 changes: 5 additions & 0 deletions Libraries/Image/Image.ios.js
View file
Open in desktop
Expand Up @@ -28,6 +28,7 @@ var invariant = require('invariant');
var requireNativeComponent = require('requireNativeComponent');
var resolveAssetSource = require('resolveAssetSource');
var warning = require('warning');
var Networking = NativeModules.Networking;

/**
* A React component for displaying different types of images,
Expand Down Expand Up @@ -151,6 +152,10 @@ var Image = React.createClass({

statics: {
resizeMode: ImageResizeMode,
/**
* Prefetch image for later use. Download remote image to the disk cache.
*/
prefetch: (url) => { Networking .prefetchImage(url); },
},

mixins: [NativeMethodsMixin],
Expand Down
6 changes: 6 additions & 0 deletions Libraries/Network/RCTNetworking.m
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@
#import "RCTHTTPRequestHandler.h"
#import "RCTLog.h"
#import "RCTUtils.h"
#import "RCTImageLoader.h"

typedef RCTURLRequestCancellationBlock (^RCTHTTPQueryResult)(NSError *error, NSDictionary<NSString *, id> *result);

Expand Down Expand Up @@ -455,6 +456,11 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request
[_tasksByRequestID removeObjectForKey:requestID];
}

RCT_EXPORT_METHOD(prefetchImage:(NSString *)url)
{
[_bridge.imageLoader loadImageWithTag:url callback:^(NSError *error, UIImage *image) {}];
}

@end

@implementation RCTBridge (RCTNetworking)
Expand Down
31 changes: 31 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java
View file
Open in desktop
Expand Up @@ -11,10 +11,17 @@

import javax.annotation.Nullable;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;

import android.net.Uri;
import com.facebook.common.logging.FLog;
import com.facebook.drawee.backends.pipeline.Fresco;
import com.facebook.imagepipeline.core.ImagePipeline;
import com.facebook.imagepipeline.request.ImageRequest;
import com.facebook.imagepipeline.request.ImageRequestBuilder;
import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.GuardedAsyncTask;
import com.facebook.react.bridge.ReactApplicationContext;
Expand All @@ -24,6 +31,7 @@
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.WritableArray;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.facebook.stetho.okhttp.StethoInterceptor;

Expand All @@ -36,6 +44,8 @@
import com.squareup.okhttp.RequestBody;
import com.squareup.okhttp.Response;
import com.squareup.okhttp.ResponseBody;
import okio.BufferedSink;
import okio.Okio;

import static java.lang.Math.min;

Expand Down Expand Up @@ -411,4 +421,25 @@ private DeviceEventManagerModule.RCTDeviceEventEmitter getEventEmitter() {
return getReactApplicationContext()
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class);
}

/**
* Prefetch image to the fresco image disk cache
*/
@ReactMethod
public void prefetchImage(String url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if url is null or empty? Add error handling.

Uri uri = null;
try {
if (url != null && !url.isEmpty()) {
uri = Uri.parse(url);
}
} catch (Exception e){
//ignore malformed url
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space after //

}
if (uri == null) {
FLog.w(ReactConstants.TAG,"Invalid prefetch image url.");
Copy link
Contributor

Choose a reason for hiding this comment

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

space after ,

Use "Invalid prefetch image url: " + url - helps with debugging.

return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can rewrite this as:

try {
  if (url != null && !url.isEmpty()) {
    Uri.parse(url);
  }
} catch (Exception ignored) {
  return;
}

But Fresco probably parses the URL anyway? What happens if you pass an invalid URL to ImageRequestBuilder or prefetchToDiskCache?

If Fresco already handles this by doing nothing, you could simply do:

if (url == null || url.isEmpty()) {
  return;
}

I'm curious - should we log a warning when people pass an empty URL to be prefetched? Seems like that would only happen by mistake and it would be a good developer experience to tell the developer about it. Look for usages of FLog.w throughout the codebase.

ImageRequest request = ImageRequestBuilder.newBuilderWithSource(uri).build();
Fresco.getImagePipeline().prefetchToDiskCache(request, this);
}
}