Skip to content

Commit

Permalink
Remove Native Extensions
Browse files Browse the repository at this point in the history
Reviewed By: michalgr

Differential Revision: D8057885

fbshipit-source-id: 6af7f7729201d26a704adaadb15813979cd035f8
  • Loading branch information
Dmitry Zakharov authored and facebook-github-bot committed May 21, 2018
1 parent 782971f commit 7c5845a
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 34 deletions.
1 change: 0 additions & 1 deletion React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ - (void)start
#if RCT_PROFILE
("StartSamplingProfilerOnInit", (bool)self.devSettings.startSamplingProfilerOnLaunch)
#endif
, nullptr
));
}
} else {
Expand Down
4 changes: 2 additions & 2 deletions ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ void injectJSCExecutorAndroidPlatform() {
}

std::unique_ptr<JSExecutorFactory> makeAndroidJSCExecutorFactory(
const folly::dynamic& jscConfig, NativeExtensionsProvider nativeExtensionsProvider) {
const folly::dynamic& jscConfig) {
detail::injectJSCExecutorAndroidPlatform();
return folly::make_unique<JSCExecutorFactory>(std::move(jscConfig), std::move(nativeExtensionsProvider));
return folly::make_unique<JSCExecutorFactory>(std::move(jscConfig));
}

}
Expand Down
2 changes: 1 addition & 1 deletion ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ void injectJSCExecutorAndroidPlatform();
}

std::unique_ptr<JSExecutorFactory> makeAndroidJSCExecutorFactory(
const folly::dynamic& jscConfig, NativeExtensionsProvider nativeExtensionsProvider);
const folly::dynamic& jscConfig);

}
}
2 changes: 1 addition & 1 deletion ReactAndroid/src/main/jni/react/jni/OnLoad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class JSCJavaScriptExecutorHolder : public HybridClass<JSCJavaScriptExecutorHold
static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/JSCJavaScriptExecutor;";

static local_ref<jhybriddata> initHybrid(alias_ref<jclass>, ReadableNativeMap* jscConfig) {
return makeCxxInstance(makeAndroidJSCExecutorFactory(jscConfig->consume(), nullptr));
return makeCxxInstance(makeAndroidJSCExecutorFactory(jscConfig->consume()));
}

static void registerNatives() {
Expand Down
25 changes: 3 additions & 22 deletions ReactCommon/cxxreact/JSCExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,17 @@ std::unique_ptr<JSExecutor> JSCExecutorFactory::createJSExecutor(
std::shared_ptr<ExecutorDelegate> delegate,
std::shared_ptr<MessageQueueThread> jsQueue) {
return folly::make_unique<JSCExecutor>(
delegate, jsQueue, m_jscConfig, m_nativeExtensionsProvider);
delegate, jsQueue, m_jscConfig);
}

JSCExecutor::JSCExecutor(
std::shared_ptr<ExecutorDelegate> delegate,
std::shared_ptr<MessageQueueThread> messageQueueThread,
const folly::dynamic& jscConfig,
NativeExtensionsProvider nativeExtensionsProvider) throw(JSException)
const folly::dynamic& jscConfig) throw(JSException)
: m_delegate(delegate),
m_messageQueueThread(messageQueueThread),
m_nativeModules(delegate ? delegate->getModuleRegistry() : nullptr),
m_jscConfig(jscConfig),
m_nativeExtensionsProvider(nativeExtensionsProvider) {
m_jscConfig(jscConfig) {
initOnJSVMThread();

{
Expand All @@ -142,12 +140,6 @@ JSCExecutor::JSCExecutor(
"nativeModuleProxy",
exceptionWrapMethod<&JSCExecutor::getNativeModule>());
}
if (nativeExtensionsProvider) {
installGlobalProxy(
m_context,
"nativeExtensions",
exceptionWrapMethod<&JSCExecutor::getNativeExtension>());
}
}

JSCExecutor::~JSCExecutor() {
Expand Down Expand Up @@ -732,17 +724,6 @@ JSValueRef JSCExecutor::getNativeModule(
return m_nativeModules.getModule(m_context, propertyName);
}

JSValueRef JSCExecutor::getNativeExtension(
JSObjectRef object,
JSStringRef propertyName) {
if (m_nativeExtensionsProvider) {
folly::dynamic value =
m_nativeExtensionsProvider(String::ref(m_context, propertyName).str());
return Value::fromDynamic(m_context, std::move(value));
}
return JSC_JSValueMakeUndefined(m_context);
}

JSValueRef JSCExecutor::nativeRequire(
size_t count,
const JSValueRef arguments[]) {
Expand Down
10 changes: 3 additions & 7 deletions ReactCommon/cxxreact/JSCExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ class RAMBundleRegistry;

class RN_EXPORT JSCExecutorFactory : public JSExecutorFactory {
public:
JSCExecutorFactory(const folly::dynamic& jscConfig, NativeExtensionsProvider provider) :
m_jscConfig(jscConfig), m_nativeExtensionsProvider(provider) {}
JSCExecutorFactory(const folly::dynamic& jscConfig) :
m_jscConfig(jscConfig) {}
std::unique_ptr<JSExecutor> createJSExecutor(
std::shared_ptr<ExecutorDelegate> delegate,
std::shared_ptr<MessageQueueThread> jsQueue) override;
private:
std::string m_cacheDir;
folly::dynamic m_jscConfig;
NativeExtensionsProvider m_nativeExtensionsProvider;
};

template<typename T>
Expand All @@ -59,8 +58,7 @@ class RN_EXPORT JSCExecutor : public JSExecutor, public PrivateDataBase {
*/
explicit JSCExecutor(std::shared_ptr<ExecutorDelegate> delegate,
std::shared_ptr<MessageQueueThread> messageQueueThread,
const folly::dynamic& jscConfig,
NativeExtensionsProvider nativeExtensionsProvider) throw(JSException);
const folly::dynamic& jscConfig) throw(JSException);
~JSCExecutor() override;

virtual void loadApplicationScript(
Expand Down Expand Up @@ -104,7 +102,6 @@ class RN_EXPORT JSCExecutor : public JSExecutor, public PrivateDataBase {
JSCNativeModules m_nativeModules;
folly::dynamic m_jscConfig;
std::once_flag m_bindFlag;
NativeExtensionsProvider m_nativeExtensionsProvider;

folly::Optional<Object> m_invokeCallbackAndReturnFlushedQueueJS;
folly::Optional<Object> m_callFunctionReturnFlushedQueueJS;
Expand All @@ -126,7 +123,6 @@ class RN_EXPORT JSCExecutor : public JSExecutor, public PrivateDataBase {
void installNativeHook(const char* name);

JSValueRef getNativeModule(JSObjectRef object, JSStringRef propertyName);
JSValueRef getNativeExtension(JSObjectRef object, JSStringRef propertyName);

JSValueRef nativeRequire(
size_t argumentCount,
Expand Down

5 comments on commit 7c5845a

@MarkOSullivan94
Copy link

Choose a reason for hiding this comment

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

Curious what this commit is doing, could someone provide a quick summary?

@mikehardy
Copy link
Contributor

Choose a reason for hiding this comment

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

The changelog for 0.56 references it like so "Remove native extensions - 7c5845a"

Does that mean there are no more native extensions for Android? What functionality was impacted, given the docs are still for 0.55 and mention Android Native Modules ? Are modules and extensions different?

@MarkOSullivan94
Copy link

Choose a reason for hiding this comment

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

I too thought it was the same thing @mikehardy which made me curious about what exactly this was doing.

@franmagneto
Copy link

Choose a reason for hiding this comment

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

I use native modules on my projects. Do I need to find alternatives to them if I want to upgrade to React Native 0.56.0? This commit sounds odd to me...

@mikehardy
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked more closely and I could be wrong but it looks like i/we are having a semantic problem with the word native. I think react has the ability to run truly native code modules (like, JNI interface on Android, implemented in C++) and that's what native means here I think. When I think of react and then think native that's different and might be called platform-specific in this context. So if I'm correct in this, native (c++ code) is still possible even, just not with extensions now. And platform-specific stuff is fine. Verify for yourself but that's my read on the related code.

Please sign in to comment.