Skip to content

Commit

Permalink
Remove MissingNativeRCTNetworkingShim; revert MissingNativeAppStateSh…
Browse files Browse the repository at this point in the history
…im (facebook#24380)

Summary:
`setupDevtools.js` is accessing `AppState.currentState` without checking its availability. In environments where 1) `__DEV__ == true`, and 2) no `RCTAppState` native module is provided thus resorting to `MissingNativeAppStateShim`, this will result in an exception:

```Cannot use 'AppState' module when native 'RCTAppState' is not included in the build. Either include it, or check 'AppState'.isAvailable before calling any methods.```

(Interestingly, `MissingNativeAppStateShim.currentState` did have a [default `null` value](facebook@118e883#diff-305b5180aa6ccc876ede6767de1fbfc4R192) that was [later removed](facebook@a93b7a2#diff-305b5180aa6ccc876ede6767de1fbfc4R186).)

**Update**: Following cpojer's suggestion of reverting facebook@a93b7a2. Title also updated to reflect this.

[General] [Fixed] - Remove MissingNativeRCTNetworkingShim; revert MissingNativeAppStateShim
Pull Request resolved: facebook#24380

Differential Revision: D14932658

Pulled By: cpojer

fbshipit-source-id: aef7ca566b3b8660eaed74a8ba3b6b0117b1200c
  • Loading branch information
statm authored and facebook-github-bot committed Apr 15, 2019
1 parent 21363ad commit 57c1a7a
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 172 deletions.
60 changes: 38 additions & 22 deletions Libraries/AppState/AppState.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

'use strict';

const MissingNativeEventEmitterShim = require('MissingNativeEventEmitterShim');
const EventEmitter = require('EventEmitter');
const NativeEventEmitter = require('NativeEventEmitter');
const NativeModules = require('NativeModules');
const RCTAppState = NativeModules.AppState;
Expand All @@ -27,7 +27,7 @@ const invariant = require('invariant');
class AppState extends NativeEventEmitter {
_eventHandlers: Object;
currentState: ?string;
isAvailable: boolean = true;
isAvailable: boolean;

constructor() {
super(RCTAppState);
Expand Down Expand Up @@ -114,32 +114,48 @@ class AppState extends NativeEventEmitter {
}
}

if (__DEV__ && !RCTAppState) {
class MissingNativeAppStateShim extends MissingNativeEventEmitterShim {
constructor() {
super('RCTAppState', 'AppState');
}
function throwMissingNativeModule() {
invariant(
false,
'Cannot use AppState module when native RCTAppState is not included in the build.\n' +
'Either include it, or check AppState.isAvailable before calling any methods.',
);
}

get currentState(): ?string {
this.throwMissingNativeModule();
}
class MissingNativeAppStateShim extends EventEmitter {
// AppState
isAvailable: boolean = false;
currentState: ?string = null;

addEventListener(...args: Array<any>) {
this.throwMissingNativeModule();
}
addEventListener() {
throwMissingNativeModule();
}

removeEventListener(...args: Array<any>) {
this.throwMissingNativeModule();
}
removeEventListener() {
throwMissingNativeModule();
}

// This module depends on the native `RCTAppState` module. If you don't
// include it, `AppState.isAvailable` will return `false`, and any method
// calls will throw. We reassign the class variable to keep the autodoc
// generator happy.
AppState = new MissingNativeAppStateShim();
} else {
// EventEmitter
addListener() {
throwMissingNativeModule();
}

removeAllListeners() {
throwMissingNativeModule();
}

removeSubscription() {
throwMissingNativeModule();
}
}

// This module depends on the native `RCTAppState` module. If you don't include it,
// `AppState.isAvailable` will return `false`, and any method calls will throw.
// We reassign the class variable to keep the autodoc generator happy.
if (RCTAppState) {
AppState = new AppState();
} else {
AppState = new MissingNativeAppStateShim();
}

module.exports = AppState;
40 changes: 18 additions & 22 deletions Libraries/Core/Devtools/setupDevtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,29 @@ let register = function() {

if (__DEV__) {
const AppState = require('AppState');
const WebSocket = require('WebSocket');
const reactDevTools = require('react-devtools-core');
const getDevServer = require('getDevServer');

// Initialize dev tools only if the native module for WebSocket is available
if (WebSocket.isAvailable) {
// Don't steal the DevTools from currently active app.
// Note: if you add any AppState subscriptions to this file,
// you will also need to guard against `AppState.isAvailable`,
// or the code will throw for bundles that don't have it.
const isAppActive = () => AppState.currentState !== 'background';
// Don't steal the DevTools from currently active app.
// Note: if you add any AppState subscriptions to this file,
// you will also need to guard against `AppState.isAvailable`,
// or the code will throw for bundles that don't have it.
const isAppActive = () => AppState.currentState !== 'background';

// Get hostname from development server (packager)
const devServer = getDevServer();
const host = devServer.bundleLoadedFromServer
? devServer.url.replace(/https?:\/\//, '').split(':')[0]
: 'localhost';
// Get hostname from development server (packager)
const devServer = getDevServer();
const host = devServer.bundleLoadedFromServer
? devServer.url.replace(/https?:\/\//, '').split(':')[0]
: 'localhost';

reactDevTools.connectToDevTools({
isAppActive,
host,
// Read the optional global variable for backward compatibility.
// It was added in https://github.com/facebook/react-native/commit/bf2b435322e89d0aeee8792b1c6e04656c2719a0.
port: window.__REACT_DEVTOOLS_PORT__,
resolveRNStyle: require('flattenStyle'),
});
}
reactDevTools.connectToDevTools({
isAppActive,
host,
// Read the optional global variable for backward compatibility.
// It was added in https://github.com/facebook/react-native/commit/bf2b435322e89d0aeee8792b1c6e04656c2719a0.
port: window.__REACT_DEVTOOLS_PORT__,
resolveRNStyle: require('flattenStyle'),
});
}

module.exports = {
Expand Down
55 changes: 0 additions & 55 deletions Libraries/EventEmitter/MissingNativeEventEmitterShim.js

This file was deleted.

32 changes: 1 addition & 31 deletions Libraries/Network/RCTNetworking.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

// Do not require the native RCTNetworking module directly! Use this wrapper module instead.
// It will add the necessary requestId, so that you don't have to generate it yourself.
const MissingNativeEventEmitterShim = require('MissingNativeEventEmitterShim');
const NativeEventEmitter = require('NativeEventEmitter');
const RCTNetworkingNative = require('NativeModules').Networking;
const convertRequestBody = require('convertRequestBody');
Expand Down Expand Up @@ -41,8 +40,6 @@ function generateRequestId(): number {
* requestId to each network request that can be used to abort that request later on.
*/
class RCTNetworking extends NativeEventEmitter {
isAvailable: boolean = true;

constructor() {
super(RCTNetworkingNative);
}
Expand Down Expand Up @@ -90,31 +87,4 @@ class RCTNetworking extends NativeEventEmitter {
}
}

if (__DEV__ && !RCTNetworkingNative) {
class MissingNativeRCTNetworkingShim extends MissingNativeEventEmitterShim {
constructor() {
super('RCTNetworking', 'Networking');
}

sendRequest(...args: Array<any>) {
this.throwMissingNativeModule();
}

abortRequest(...args: Array<any>) {
this.throwMissingNativeModule();
}

clearCookies(...args: Array<any>) {
this.throwMissingNativeModule();
}
}

// This module depends on the native `RCTNetworkingNative` module. If you don't include it,
// `RCTNetworking.isAvailable` will return `false`, and any method calls will throw.
// We reassign the class variable to keep the autodoc generator happy.
RCTNetworking = new MissingNativeRCTNetworkingShim();
} else {
RCTNetworking = new RCTNetworking();
}

module.exports = RCTNetworking;
module.exports = new RCTNetworking();
32 changes: 1 addition & 31 deletions Libraries/Network/RCTNetworking.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

'use strict';

const MissingNativeEventEmitterShim = require('MissingNativeEventEmitterShim');
const NativeEventEmitter = require('NativeEventEmitter');
const RCTNetworkingNative = require('NativeModules').Networking;
const convertRequestBody = require('convertRequestBody');
Expand All @@ -20,8 +19,6 @@ import type {RequestBody} from 'convertRequestBody';
import type {NativeResponseType} from './XMLHttpRequest';

class RCTNetworking extends NativeEventEmitter {
isAvailable: boolean = true;

constructor() {
super(RCTNetworkingNative);
}
Expand Down Expand Up @@ -63,31 +60,4 @@ class RCTNetworking extends NativeEventEmitter {
}
}

if (__DEV__ && !RCTNetworkingNative) {
class MissingNativeRCTNetworkingShim extends MissingNativeEventEmitterShim {
constructor() {
super('RCTNetworking', 'Networking');
}

sendRequest(...args: Array<any>) {
this.throwMissingNativeModule();
}

abortRequest(...args: Array<any>) {
this.throwMissingNativeModule();
}

clearCookies(...args: Array<any>) {
this.throwMissingNativeModule();
}
}

// This module depends on the native `RCTNetworkingNative` module. If you don't include it,
// `RCTNetworking.isAvailable` will return `false`, and any method calls will throw.
// We reassign the class variable to keep the autodoc generator happy.
RCTNetworking = new MissingNativeRCTNetworkingShim();
} else {
RCTNetworking = new RCTNetworking();
}

module.exports = RCTNetworking;
module.exports = new RCTNetworking();
11 changes: 0 additions & 11 deletions Libraries/WebSocket/WebSocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ class WebSocket extends EventTarget(...WEBSOCKET_EVENTS) {
readyState: number = CONNECTING;
url: ?string;

// This module depends on the native `WebSocketModule` module. If you don't include it,
// `WebSocket.isAvailable` will return `false`, and WebSocket constructor will throw an error
static isAvailable: boolean = !!WebSocketModule;

constructor(
url: string,
protocols: ?string | ?Array<string>,
Expand Down Expand Up @@ -132,13 +128,6 @@ class WebSocket extends EventTarget(...WEBSOCKET_EVENTS) {
protocols = null;
}

if (!WebSocket.isAvailable) {
throw new Error(
'Cannot initialize WebSocket module. ' +
'Native module WebSocketModule is missing.',
);
}

this._eventEmitter = new NativeEventEmitter(WebSocketModule);
this._socketId = nextWebSocketId++;
this._registerEvents();
Expand Down

0 comments on commit 57c1a7a

Please sign in to comment.