Skip to content

Commit

Permalink
Breaking - remove unused registration of JS modules
Browse files Browse the repository at this point in the history 
Summary: It's now unnecessary to declare which JS modules you want to expose on your package. To upgrade, remove all overrides of `createJSModules` and keeping calling your JS modules as before.

Reviewed By: AaaChiuuu

Differential Revision: D5229259

fbshipit-source-id: 1160826c951433722f1fe0421c1200883ba1a348
  • Loading branch information
@javache @facebook-github-bot
javache authored and facebook-github-bot committed Jun 14, 2017
1 parent 71ea94b commit ce6fb33
Show file tree
Hide file tree
Showing 30 changed files with 15 additions and 203 deletions.
3 changes: 1 addition & 2 deletions ReactAndroid/src/androidTest/java/com/facebook/react/testing/AbstractScrollViewTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ protected void tearDown() throws Exception {
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
mScrollListenerModule = new ScrollListenerModule();
return super.createReactInstanceSpecForTest()
.addNativeModule(mScrollListenerModule)
.addJSModule(ScrollViewTestModule.class);
.addNativeModule(mScrollListenerModule);
}

// See ScrollViewListenerModule.js
Expand Down
5 changes: 0 additions & 5 deletions ReactAndroid/src/androidTest/java/com/facebook/react/testing/InstanceSpecForTestPackage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ public List<NativeModule> createNativeModules(
return mSpecForTest.getExtraNativeModulesForTest();
}

@Override
public List<Class<? extends JavaScriptModule>> createJSModules() {
return mSpecForTest.getExtraJSModulesForTest();
}

@Override
public List<ViewManager> createViewManagers(ReactApplicationContext reactContext) {
return mSpecForTest.getExtraViewManagers();
Expand Down
9 changes: 0 additions & 9 deletions ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstanceSpecForTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ public ReactInstanceSpecForTest addNativeModule(NativeModule module) {
return this;
}

public ReactInstanceSpecForTest addJSModule(Class jsClass) {
mJSModuleSpecs.add(jsClass);
return this;
}

public ReactInstanceSpecForTest setPackage(ReactPackage reactPackage) {
mReactPackage = reactPackage;
return this;
Expand All @@ -57,10 +52,6 @@ public List<NativeModule> getExtraNativeModulesForTest() {
return mNativeModules;
}

public List<Class<? extends JavaScriptModule>> getExtraJSModulesForTest() {
return mJSModuleSpecs;
}

public ReactPackage getAlternativeReactPackageForTest() {
return mReactPackage;
}
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public interface ReactTestFactory {
public static interface ReactInstanceEasyBuilder {
ReactInstanceEasyBuilder setContext(Context context);
ReactInstanceEasyBuilder addNativeModule(NativeModule module);
ReactInstanceEasyBuilder addJSModule(Class moduleInterfaceClass);
CatalystInstance build();
}

Expand Down
17 changes: 2 additions & 15 deletions ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import android.view.View;
import android.view.ViewGroup;

import com.facebook.infer.annotation.Assertions;
import com.facebook.react.NativeModuleRegistryBuilder;
import com.facebook.react.R;
import com.facebook.react.ReactInstanceManager;
Expand All @@ -39,8 +40,6 @@ public class ReactTestHelper {
private static class DefaultReactTestFactory implements ReactTestFactory {
private static class ReactInstanceEasyBuilderImpl implements ReactInstanceEasyBuilder {

private final JavaScriptModuleRegistry.Builder mJSModuleRegistryBuilder =
new JavaScriptModuleRegistry.Builder();
private NativeModuleRegistryBuilder mNativeModuleRegistryBuilder;

private @Nullable Context mContext;
Expand All @@ -59,16 +58,11 @@ public ReactInstanceEasyBuilder addNativeModule(NativeModule nativeModule) {
null,
false);
}
Assertions.assertNotNull(nativeModule);
mNativeModuleRegistryBuilder.addNativeModule(nativeModule);
return this;
}

@Override
public ReactInstanceEasyBuilder addJSModule(Class moduleInterfaceClass) {
mJSModuleRegistryBuilder.add(moduleInterfaceClass);
return this;
}

@Override
public CatalystInstance build() {
if (mNativeModuleRegistryBuilder == null) {
Expand All @@ -87,7 +81,6 @@ public CatalystInstance build() {
.setReactQueueConfigurationSpec(ReactQueueConfigurationSpec.createDefault())
.setJSExecutor(executor)
.setRegistry(mNativeModuleRegistryBuilder.build())
.setJSModuleRegistry(mJSModuleRegistryBuilder.build())
.setJSBundleLoader(JSBundleLoader.createAssetLoader(
mContext,
"assets://AndroidTestBundle.js",
Expand Down Expand Up @@ -141,12 +134,6 @@ public ReactTestFactory.ReactInstanceEasyBuilder addNativeModule(NativeModule mo
return this;
}

@Override
public ReactTestFactory.ReactInstanceEasyBuilder addJSModule(Class moduleInterfaceClass) {
builder.addJSModule(moduleInterfaceClass);
return this;
}

@Override
public CatalystInstance build() {
final CatalystInstance instance = builder.build();
Expand Down
5 changes: 2 additions & 3 deletions ReactAndroid/src/androidTest/java/com/facebook/react/tests/CatalystMeasureLayoutTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ protected String getReactApplicationKeyUnderTest() {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
mAssertModule = new AssertModule();
return new ReactInstanceSpecForTest()
.addNativeModule(mAssertModule)
.addJSModule(MeasureLayoutTestModule.class);
return super.createReactInstanceSpecForTest()
.addNativeModule(mAssertModule);
}

private void waitForBridgeIdleAndVerifyAsserts() {
Expand Down
1 change: 0 additions & 1 deletion ...c/androidTest/java/com/facebook/react/tests/CatalystNativeJSToJavaParametersTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public void run() {
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addNativeModule(mUIManager)
.addJSModule(TestJSToJavaParametersModule.class)
.build();
}

Expand Down
1 change: 0 additions & 1 deletion ...rc/androidTest/java/com/facebook/react/tests/CatalystNativeJavaToJSArgumentsTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ public void run() {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(TestJavaToJSArgumentsModule.class)
.addNativeModule(mUIManager)
.build();
}
Expand Down
1 change: 0 additions & 1 deletion ...androidTest/java/com/facebook/react/tests/CatalystNativeJavaToJSReturnValuesTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ protected void setUp() throws Exception {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(TestJavaToJSReturnValuesModule.class)
.addNativeModule(mUIManager)
.addNativeModule(new TestModule())
.build();
Expand Down
6 changes: 2 additions & 4 deletions ...droid/src/androidTest/java/com/facebook/react/tests/CatalystSubviewsClippingTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,8 @@ protected String getReactApplicationKeyUnderTest() {

@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
ReactInstanceSpecForTest instanceSpec = new ReactInstanceSpecForTest();
instanceSpec.addJSModule(SubviewsClippingTestModule.class);
instanceSpec.addViewManager(new ClippableViewManager(mEvents));
return instanceSpec;
return super.createReactInstanceSpecForTest()
.addViewManager(new ClippableViewManager(mEvents));
}

/**
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/tests/CatalystUIManagerTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ public void run() {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(UIManagerTestModule.class)
.build()
.getJSModule(UIManagerTestModule.class);
}
Expand Down
3 changes: 1 addition & 2 deletions ReactAndroid/src/androidTest/java/com/facebook/react/tests/DatePickerDialogTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ public int getErrors() {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
return super.createReactInstanceSpecForTest()
.addNativeModule(mRecordingModule)
.addJSModule(DatePickerDialogTestModule.class);
.addNativeModule(mRecordingModule);
}

@Override
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/tests/JSLocaleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ public void run() {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(TestJSLocaleModule.class)
.build();
}

Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/tests/ProgressBarTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ public void run() {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(ProgressBarTestModule.class)
.build();

mRootView = new ReactRootView(getContext());
Expand Down
3 changes: 1 addition & 2 deletions ReactAndroid/src/androidTest/java/com/facebook/react/tests/ReactPickerTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

package com.facebook.react.tests;

import java.util.ArrayList;
Expand Down Expand Up @@ -68,7 +68,6 @@ protected String getReactApplicationKeyUnderTest() {
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
mRecordingModule = new PickerAndroidRecordingModule();
return super.createReactInstanceSpecForTest()
.addJSModule(PickerAndroidTestModule.class)
.addNativeModule(mRecordingModule);
}

Expand Down
3 changes: 1 addition & 2 deletions ...ndroid/src/androidTest/java/com/facebook/react/tests/ReactSwipeRefreshLayoutTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ protected String getReactApplicationKeyUnderTest() {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
return super.createReactInstanceSpecForTest()
.addNativeModule(mRecordingModule)
.addJSModule(SwipeRefreshLayoutTestModule.class);
.addNativeModule(mRecordingModule);
}

public void testRefreshNoScroll() {
Expand Down
3 changes: 1 addition & 2 deletions ReactAndroid/src/androidTest/java/com/facebook/react/tests/ShareTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ public int getErrors() {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
return super.createReactInstanceSpecForTest()
.addNativeModule(mRecordingModule)
.addJSModule(ShareTestModule.class);
.addNativeModule(mRecordingModule);
}

@Override
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/tests/TextInputTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ public void testMetionsInputColors() throws Throwable {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
return super.createReactInstanceSpecForTest()
.addJSModule(TextInputTestModule.class)
.addNativeModule(mRecordingModule);
}

Expand Down
3 changes: 1 addition & 2 deletions ReactAndroid/src/androidTest/java/com/facebook/react/tests/TimePickerDialogTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public int getErrors() {
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
return super.createReactInstanceSpecForTest()
.addNativeModule(mRecordingModule)
.addJSModule(TimePickerDialogTestModule.class);
.addNativeModule(mRecordingModule);
}

@Override
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/androidTest/java/com/facebook/react/tests/ViewRenderingTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public void run() {
.addNativeModule(new DeviceInfoModule(getContext()))
.addNativeModule(new AppStateModule(getContext()))
.addNativeModule(new FakeWebSocketModule())
.addJSModule(ViewRenderingTestModule.class)
.build();

mRootView = new ReactRootView(getContext());
Expand Down
14 changes: 0 additions & 14 deletions ReactAndroid/src/main/java/com/facebook/react/CompositeReactPackage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,6 @@ public List<NativeModule> createNativeModules(
return new ArrayList(moduleMap.values());
}

/**
* {@inheritDoc}
*/
@Override
public List<Class<? extends JavaScriptModule>> createJSModules() {
final Set<Class<? extends JavaScriptModule>> moduleSet = new HashSet<>();
for (ReactPackage reactPackage: mChildReactPackages) {
for (Class<? extends JavaScriptModule> jsModule: reactPackage.createJSModules()) {
moduleSet.add(jsModule);
}
}
return new ArrayList(moduleSet);
}

/**
* {@inheritDoc}
*/
Expand Down
27 changes: 0 additions & 27 deletions ReactAndroid/src/main/java/com/facebook/react/CoreModulesPackage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,21 @@
import javax.inject.Provider;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import com.facebook.react.bridge.JavaScriptModule;
import com.facebook.react.bridge.ModuleSpec;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactMarker;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.devsupport.HMRClient;
import com.facebook.react.devsupport.JSCHeapCapture;
import com.facebook.react.devsupport.JSCSamplingProfiler;
import com.facebook.react.module.annotations.ReactModuleList;
import com.facebook.react.module.model.ReactModuleInfoProvider;
import com.facebook.react.modules.appregistry.AppRegistry;
import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.facebook.react.modules.core.ExceptionsManagerModule;
import com.facebook.react.modules.core.HeadlessJsTaskSupportModule;
import com.facebook.react.modules.core.JSTimersExecution;
import com.facebook.react.modules.core.RCTNativeAppEventEmitter;
import com.facebook.react.modules.core.Timing;
import com.facebook.react.modules.debug.AnimationsDebugModule;
import com.facebook.react.modules.debug.SourceCodeModule;
Expand All @@ -42,7 +36,6 @@
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.ViewManager;
import com.facebook.react.uimanager.debug.DebugComponentOwnershipModule;
import com.facebook.react.uimanager.events.RCTEventEmitter;
import com.facebook.systrace.Systrace;

import static com.facebook.react.bridge.ReactMarkerConstants.CREATE_UI_MANAGER_MODULE_END;
Expand Down Expand Up @@ -187,26 +180,6 @@ public NativeModule get() {
return moduleSpecList;
}

@Override
public List<Class<? extends JavaScriptModule>> createJSModules() {
List<Class<? extends JavaScriptModule>> jsModules = new ArrayList<>(Arrays.asList(
DeviceEventManagerModule.RCTDeviceEventEmitter.class,
JSTimersExecution.class,
RCTEventEmitter.class,
RCTNativeAppEventEmitter.class,
AppRegistry.class,
com.facebook.react.bridge.Systrace.class,
HMRClient.class));

if (ReactBuildConfig.DEBUG) {
jsModules.add(DebugComponentOwnershipModule.RCTDebugComponentOwnership.class);
jsModules.add(JSCHeapCapture.HeapCapture.class);
jsModules.add(JSCSamplingProfiler.SamplingProfiler.class);
}

return jsModules;
}

@Override
public ReactModuleInfoProvider getReactModuleInfoProvider() {
// This has to be done via reflection or we break open source.
Expand Down
12 changes: 3 additions & 9 deletions ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,6 @@ private ReactApplicationContext createReactContext(
reactContext,
this,
mLazyNativeModulesEnabled);
JavaScriptModuleRegistry.Builder jsModulesBuilder = new JavaScriptModuleRegistry.Builder();
if (mUseDeveloperSupport) {
reactContext.setNativeModuleCallExceptionHandler(mDevSupportManager);
}
Expand All @@ -942,7 +941,7 @@ private ReactApplicationContext createReactContext(
mBackBtnHandler,
mUIImplementationProvider,
mLazyViewManagersEnabled);
processPackage(coreModulesPackage, nativeModuleRegistryBuilder, jsModulesBuilder);
processPackage(coreModulesPackage, nativeModuleRegistryBuilder);
} finally {
Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
}
Expand All @@ -953,7 +952,7 @@ private ReactApplicationContext createReactContext(
TRACE_TAG_REACT_JAVA_BRIDGE,
"createAndProcessCustomReactPackage");
try {
processPackage(reactPackage, nativeModuleRegistryBuilder, jsModulesBuilder);
processPackage(reactPackage, nativeModuleRegistryBuilder);
} finally {
Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
}
Expand All @@ -979,7 +978,6 @@ private ReactApplicationContext createReactContext(
ReactQueueConfigurationSpec.createDefault())
.setJSExecutor(jsExecutor)
.setRegistry(nativeModuleRegistry)
.setJSModuleRegistry(jsModulesBuilder.build())
.setJSBundleLoader(jsBundleLoader)
.setNativeModuleCallExceptionHandler(exceptionHandler);

Expand Down Expand Up @@ -1010,8 +1008,7 @@ private ReactApplicationContext createReactContext(

private void processPackage(
ReactPackage reactPackage,
NativeModuleRegistryBuilder nativeModuleRegistryBuilder,
JavaScriptModuleRegistry.Builder jsModulesBuilder) {
NativeModuleRegistryBuilder nativeModuleRegistryBuilder) {
SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "processPackage")
.arg("className", reactPackage.getClass().getSimpleName())
.flush();
Expand All @@ -1020,9 +1017,6 @@ private void processPackage(
}
nativeModuleRegistryBuilder.processPackage(reactPackage);

for (Class<? extends JavaScriptModule> jsModuleClass : reactPackage.createJSModules()) {
jsModulesBuilder.add(jsModuleClass);
}
if (reactPackage instanceof ReactPackageLogger) {
((ReactPackageLogger) reactPackage).endProcessPackage();
}
Expand Down
Loading

3 comments on commit ce6fb33

@ujwal-setlur
Copy link

Choose a reason for hiding this comment

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

This is causing compile breakage of a lot of third party modules. This is because they override createJSModules.

@krunalsshah
Copy link

Choose a reason for hiding this comment

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

Can you please let us know what's the best practice for a case where we want to support react-native version 0.42 & upgrade to 0.48? Update the minor version with this breaking change?

@CesarLanderos
Copy link

@CesarLanderos CesarLanderos commented on ce6fb33 Nov 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is what i did, should be safe for everyone:

  1. If a package breaks with this react-native version, do not upgrade your react-native version.
  2. If you know for sure that this breaking change is causing an specific package to brake, go to that package repo and open a PR (follow this example: Fix android breaking change RN >= 0.47 CORBmx/react-native-openpay#3) and tell the mantainers that this is a breaking change, so they can publish a new version with that change (which shoul include increasing the major version of that package).
  3. Bump your own app major version (if you are using semver, if not, do something similar for whichever schema you are using) that includes both bumps of RN >= 0.47 and the now fixed third party package.

that is my own recommendation 😬

Please sign in to comment.