Skip to content

Commit

Permalink
Split JS spec for image loader module
Browse files Browse the repository at this point in the history
Summary:
It turns out the ImageLoader native module has different method signatures on iOS than on Android, so the JS spec we currently have won't work for ANdroid. In this diff I'm splitting up the spec for NativeImageLoader into an Android & iOS versions (similar to PlatformConstants), and updating the Android spec to match the native implementation. I'm also changing `RCTImageLoader` to use the new generated spec, and updating the JS callers (`Image.android.js` and `Image.ios.js`) to use the right one for the platform (instead of importing the untyped `ImageLoader` native module from `react-native`, like we were on Android :-/).

This will be a breaking change for anyone who's directly using `NativeImageLoader.js`, but I think most callsites should be using the `Image` component instead.

Changelog: [General] [Changed] Split NativeImageLoader into NativeImageLoaderAndroid and NativeImageLoaderIOS

Reviewed By: RSNara

Differential Revision: D18439538

fbshipit-source-id: 94c796d3fd27800ea17053e963bee51aca921718
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Nov 12, 2019
1 parent 390546f commit d37baa7
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1325,36 +1325,84 @@ + (RCTManagedPointer *)JS_NativeImageEditor_Options:(id)json
namespace react {


static facebook::jsi::Value __hostFunction_NativeImageLoaderSpecJSI_getSize(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
static facebook::jsi::Value __hostFunction_NativeImageLoaderAndroidSpecJSI_abortRequest(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, VoidKind, "abortRequest", @selector(abortRequest:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderAndroidSpecJSI_getSize(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "getSize", @selector(getSize:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderAndroidSpecJSI_getSizeWithHeaders(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "getSizeWithHeaders", @selector(getSizeWithHeaders:headers:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderAndroidSpecJSI_prefetchImage(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "prefetchImage", @selector(prefetchImage:requestId:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderAndroidSpecJSI_queryCache(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "queryCache", @selector(queryCache:resolve:reject:), args, count);
}


NativeImageLoaderAndroidSpecJSI::NativeImageLoaderAndroidSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker)
: ObjCTurboModule("ImageLoaderAndroid", instance, jsInvoker) {

methodMap_["abortRequest"] = MethodMetadata {1, __hostFunction_NativeImageLoaderAndroidSpecJSI_abortRequest};


methodMap_["getSize"] = MethodMetadata {1, __hostFunction_NativeImageLoaderAndroidSpecJSI_getSize};


methodMap_["getSizeWithHeaders"] = MethodMetadata {2, __hostFunction_NativeImageLoaderAndroidSpecJSI_getSizeWithHeaders};


methodMap_["prefetchImage"] = MethodMetadata {2, __hostFunction_NativeImageLoaderAndroidSpecJSI_prefetchImage};


methodMap_["queryCache"] = MethodMetadata {1, __hostFunction_NativeImageLoaderAndroidSpecJSI_queryCache};



}

} // namespace react
} // namespace facebook
namespace facebook {
namespace react {


static facebook::jsi::Value __hostFunction_NativeImageLoaderIOSSpecJSI_getSize(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "getSize", @selector(getSize:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderSpecJSI_getSizeWithHeaders(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
static facebook::jsi::Value __hostFunction_NativeImageLoaderIOSSpecJSI_getSizeWithHeaders(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "getSizeWithHeaders", @selector(getSizeWithHeaders:headers:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderSpecJSI_prefetchImage(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
static facebook::jsi::Value __hostFunction_NativeImageLoaderIOSSpecJSI_prefetchImage(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "prefetchImage", @selector(prefetchImage:resolve:reject:), args, count);
}

static facebook::jsi::Value __hostFunction_NativeImageLoaderSpecJSI_queryCache(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
static facebook::jsi::Value __hostFunction_NativeImageLoaderIOSSpecJSI_queryCache(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, "queryCache", @selector(queryCache:resolve:reject:), args, count);
}


NativeImageLoaderSpecJSI::NativeImageLoaderSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker)
: ObjCTurboModule("ImageLoader", instance, jsInvoker) {
NativeImageLoaderIOSSpecJSI::NativeImageLoaderIOSSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker)
: ObjCTurboModule("ImageLoaderIOS", instance, jsInvoker) {

methodMap_["getSize"] = MethodMetadata {1, __hostFunction_NativeImageLoaderSpecJSI_getSize};
methodMap_["getSize"] = MethodMetadata {1, __hostFunction_NativeImageLoaderIOSSpecJSI_getSize};


methodMap_["getSizeWithHeaders"] = MethodMetadata {2, __hostFunction_NativeImageLoaderSpecJSI_getSizeWithHeaders};
methodMap_["getSizeWithHeaders"] = MethodMetadata {2, __hostFunction_NativeImageLoaderIOSSpecJSI_getSizeWithHeaders};


methodMap_["prefetchImage"] = MethodMetadata {1, __hostFunction_NativeImageLoaderSpecJSI_prefetchImage};
methodMap_["prefetchImage"] = MethodMetadata {1, __hostFunction_NativeImageLoaderIOSSpecJSI_prefetchImage};


methodMap_["queryCache"] = MethodMetadata {1, __hostFunction_NativeImageLoaderSpecJSI_queryCache};
methodMap_["queryCache"] = MethodMetadata {1, __hostFunction_NativeImageLoaderIOSSpecJSI_queryCache};



Expand Down
40 changes: 36 additions & 4 deletions Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,39 @@ namespace facebook {
};
} // namespace react
} // namespace facebook
@protocol NativeImageLoaderSpec <RCTBridgeModule, RCTTurboModule>
@protocol NativeImageLoaderAndroidSpec <RCTBridgeModule, RCTTurboModule>

- (void)abortRequest:(double)requestId;
- (void)getSize:(NSString *)uri
resolve:(RCTPromiseResolveBlock)resolve
reject:(RCTPromiseRejectBlock)reject;
- (void)getSizeWithHeaders:(NSString *)uri
headers:(NSDictionary *)headers
resolve:(RCTPromiseResolveBlock)resolve
reject:(RCTPromiseRejectBlock)reject;
- (void)prefetchImage:(NSString *)uri
requestId:(double)requestId
resolve:(RCTPromiseResolveBlock)resolve
reject:(RCTPromiseRejectBlock)reject;
- (void)queryCache:(NSArray *)uris
resolve:(RCTPromiseResolveBlock)resolve
reject:(RCTPromiseRejectBlock)reject;

@end
namespace facebook {
namespace react {
/**
* ObjC++ class for module 'ImageLoaderAndroid'
*/

class JSI_EXPORT NativeImageLoaderAndroidSpecJSI : public ObjCTurboModule {
public:
NativeImageLoaderAndroidSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker);

};
} // namespace react
} // namespace facebook
@protocol NativeImageLoaderIOSSpec <RCTBridgeModule, RCTTurboModule>

- (void)getSize:(NSString *)uri
resolve:(RCTPromiseResolveBlock)resolve
Expand All @@ -1486,12 +1518,12 @@ namespace facebook {
namespace facebook {
namespace react {
/**
* ObjC++ class for module 'ImageLoader'
* ObjC++ class for module 'ImageLoaderIOS'
*/

class JSI_EXPORT NativeImageLoaderSpecJSI : public ObjCTurboModule {
class JSI_EXPORT NativeImageLoaderIOSSpecJSI : public ObjCTurboModule {
public:
NativeImageLoaderSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker);
NativeImageLoaderIOSSpecJSI(id<RCTTurboModule> instance, std::shared_ptr<CallInvoker> jsInvoker);

};
} // namespace react
Expand Down
13 changes: 6 additions & 7 deletions Libraries/Image/Image.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const DeprecatedImageStylePropTypes = require('../DeprecatedPropTypes/Deprecated
const DeprecatedStyleSheetPropType = require('../DeprecatedPropTypes/DeprecatedStyleSheetPropType');
const DeprecatedViewPropTypes = require('../DeprecatedPropTypes/DeprecatedViewPropTypes');
const ImageViewNativeComponent = require('./ImageViewNativeComponent');
const NativeModules = require('../BatchedBridge/NativeModules');
const PropTypes = require('prop-types');
const React = require('react');
const ReactNative = require('../Renderer/shims/ReactNative'); // eslint-disable-line no-unused-vars
Expand All @@ -24,7 +23,7 @@ const TextAncestor = require('../Text/TextAncestor');
const flattenStyle = require('../StyleSheet/flattenStyle');
const resolveAssetSource = require('./resolveAssetSource');

const {ImageLoader} = NativeModules;
import NativeImageLoaderAndroid from './NativeImageLoaderAndroid';

const TextInlineImageNativeComponent = require('./TextInlineImageNativeComponent');

Expand Down Expand Up @@ -149,7 +148,7 @@ function getSize(
success: (width: number, height: number) => void,
failure?: (error: any) => void,
): any {
return ImageLoader.getSize(url)
return NativeImageLoaderAndroid.getSize(url)
.then(function(sizes) {
success(sizes.width, sizes.height);
})
Expand All @@ -173,7 +172,7 @@ function getSizeWithHeaders(
success: (width: number, height: number) => void,
failure?: (error: any) => void,
): any {
return ImageLoader.getSizeWithHeaders(url, headers)
return NativeImageLoaderAndroid.getSizeWithHeaders(url, headers)
.then(function(sizes) {
success(sizes.width, sizes.height);
})
Expand All @@ -188,11 +187,11 @@ function getSizeWithHeaders(
function prefetch(url: string, callback: ?Function): any {
const requestId = generateRequestId();
callback && callback(requestId);
return ImageLoader.prefetchImage(url, requestId);
return NativeImageLoaderAndroid.prefetchImage(url, requestId);
}

function abortPrefetch(requestId: number) {
ImageLoader.abortRequest(requestId);
NativeImageLoaderAndroid.abortRequest(requestId);
}

/**
Expand All @@ -203,7 +202,7 @@ function abortPrefetch(requestId: number) {
async function queryCache(
urls: Array<string>,
): Promise<{[string]: 'memory' | 'disk' | 'disk/memory'}> {
return await ImageLoader.queryCache(urls);
return await NativeImageLoaderAndroid.queryCache(urls);
}

type ImageComponentStatics = $ReadOnly<{|
Expand Down
10 changes: 5 additions & 5 deletions Libraries/Image/Image.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import type {ImageProps as ImagePropsType} from './ImageProps';
import type {HostComponent} from '../Renderer/shims/ReactNativeTypes';

import type {ImageStyleProp} from '../StyleSheet/StyleSheet';
import NativeImageLoader from './NativeImageLoader';
import NativeImageLoaderIOS from './NativeImageLoaderIOS';

const ImageViewManager = NativeModules.ImageViewManager;
const RCTImageView: HostComponent<mixed> = requireNativeComponent(
Expand All @@ -36,7 +36,7 @@ function getSize(
success: (width: number, height: number) => void,
failure?: (error: any) => void,
) {
NativeImageLoader.getSize(uri)
NativeImageLoaderIOS.getSize(uri)
.then(([width, height]) => success(width, height))
.catch(
failure ||
Expand All @@ -52,7 +52,7 @@ function getSizeWithHeaders(
success: (width: number, height: number) => void,
failure?: (error: any) => void,
): any {
return NativeImageLoader.getSizeWithHeaders(uri, headers)
return NativeImageLoaderIOS.getSizeWithHeaders(uri, headers)
.then(function(sizes) {
success(sizes.width, sizes.height);
})
Expand All @@ -65,13 +65,13 @@ function getSizeWithHeaders(
}

function prefetch(url: string): any {
return NativeImageLoader.prefetchImage(url);
return NativeImageLoaderIOS.prefetchImage(url);
}

async function queryCache(
urls: Array<string>,
): Promise<{[string]: 'memory' | 'disk' | 'disk/memory'}> {
return await NativeImageLoader.queryCache(urls);
return await NativeImageLoaderIOS.queryCache(urls);
}

type ImageComponentStatics = $ReadOnly<{|
Expand Down
30 changes: 30 additions & 0 deletions Libraries/Image/NativeImageLoaderAndroid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
* @format
*/

'use strict';

import type {TurboModule} from '../TurboModule/RCTExport';
import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry';

export interface Spec extends TurboModule {
+abortRequest: (requestId: number) => void;
+getConstants: () => {||};
+getSize: (
uri: string,
) => Promise<$ReadOnly<{width: number, height: number}>>;
+getSizeWithHeaders: (
uri: string,
headers: Object,
) => Promise<{width: number, height: number}>;
+prefetchImage: (uri: string, requestId: number) => Promise<boolean>;
+queryCache: (uris: Array<string>) => Promise<Object>;
}

export default (TurboModuleRegistry.getEnforcing<Spec>('ImageLoader'): Spec);
File renamed without changes.
4 changes: 2 additions & 2 deletions Libraries/Image/RCTImageLoader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static NSInteger RCTImageBytesForImage(UIImage *image)
return image.images ? image.images.count * singleImageBytes : singleImageBytes;
}

@interface RCTImageLoader() <NativeImageLoaderSpec>
@interface RCTImageLoader() <NativeImageLoaderIOSSpec>

@end

Expand Down Expand Up @@ -946,7 +946,7 @@ - (void)cancelRequest:(id)requestToken
- (std::shared_ptr<facebook::react::TurboModule>)getTurboModuleWithJsInvoker:
(std::shared_ptr<facebook::react::CallInvoker>)jsInvoker
{
return std::make_shared<facebook::react::NativeImageLoaderSpecJSI>(self, jsInvoker);
return std::make_shared<facebook::react::NativeImageLoaderIOSSpecJSI>(self, jsInvoker);
}

RCT_EXPORT_METHOD(getSize:(NSString *)uri resolve:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* <p>This source code is licensed under the MIT license found in the LICENSE file in the root
* directory of this source tree.
*
* <p>Generated by an internal genrule from Flow types.
*
* @generated
* @nolint
*/

package com.facebook.fbreact.specs;

import com.facebook.react.bridge.Promise;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactMethod;
import com.facebook.react.bridge.ReactModuleWithSpec;
import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;

public abstract class NativeImageLoaderAndroidSpec extends ReactContextBaseJavaModule implements ReactModuleWithSpec, TurboModule {
public NativeImageLoaderAndroidSpec(ReactApplicationContext reactContext) {
super(reactContext);
}

@ReactMethod
public abstract void getSize(String uri, Promise promise);

@ReactMethod
public abstract void abortRequest(double requestId);

@ReactMethod
public abstract void prefetchImage(String uri, double requestId, Promise promise);

@ReactMethod
public abstract void queryCache(ReadableArray uris, Promise promise);

@ReactMethod
public abstract void getSizeWithHeaders(String uri, ReadableMap headers, Promise promise);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* <p>This source code is licensed under the MIT license found in the LICENSE file in the root
* directory of this source tree.
*
* <p>Generated by an internal genrule from Flow types.
*
* @generated
* @nolint
*/

package com.facebook.fbreact.specs;

import com.facebook.react.bridge.Promise;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactMethod;
import com.facebook.react.bridge.ReactModuleWithSpec;
import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;

public abstract class NativeImageLoaderIOSSpec extends ReactContextBaseJavaModule implements ReactModuleWithSpec, TurboModule {
public NativeImageLoaderIOSSpec(ReactApplicationContext reactContext) {
super(reactContext);
}

@ReactMethod
public abstract void getSize(String uri, Promise promise);

@ReactMethod
public abstract void prefetchImage(String uri, Promise promise);

@ReactMethod
public abstract void queryCache(ReadableArray uris, Promise promise);

@ReactMethod
public abstract void getSizeWithHeaders(String uri, ReadableMap headers, Promise promise);
}

0 comments on commit d37baa7

Please sign in to comment.