From 2dcdc3c633809d176ca6716dca1bddcd7d835be9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 13 Jul 2017 20:44:52 +0100 Subject: [PATCH] Simplify environment injections (#10175) * Remove unnecessary injection guard * Remove inject() indirection for global injections It was necessary when they shared global state. But now these are flat bundles so we can inject as side effect. This feels a bit less convoluted to me. Ideally we can get rid of this somehow soon. --- src/renderers/dom/ReactDOMNodeStreamEntry.js | 3 +- src/renderers/dom/ReactDOMServerEntry.js | 3 +- src/renderers/dom/ReactDOMServerStackEntry.js | 6 +- src/renderers/dom/ReactDOMStackEntry.js | 9 +-- src/renderers/dom/fiber/ReactDOMFiberEntry.js | 6 +- .../dom/shared/ReactDOMClientInjection.js | 54 +++++-------- src/renderers/dom/shared/ReactDOMInjection.js | 22 +----- .../__tests__/DOMPropertyOperations-test.js | 3 +- .../stack/client/ReactDOMStackInjection.js | 54 +++++-------- src/renderers/native/ReactNativeFiberEntry.js | 3 +- src/renderers/native/ReactNativeInjection.js | 46 +++++------ src/renderers/native/ReactNativeStackEntry.js | 6 +- .../native/ReactNativeStackInjection.js | 78 +++++++++---------- 13 files changed, 109 insertions(+), 184 deletions(-) diff --git a/src/renderers/dom/ReactDOMNodeStreamEntry.js b/src/renderers/dom/ReactDOMNodeStreamEntry.js index b7f2cb5f58..9353ef804c 100644 --- a/src/renderers/dom/ReactDOMNodeStreamEntry.js +++ b/src/renderers/dom/ReactDOMNodeStreamEntry.js @@ -11,11 +11,10 @@ 'use strict'; -var ReactDOMInjection = require('ReactDOMInjection'); var ReactDOMNodeStreamRenderer = require('ReactDOMNodeStreamRenderer'); var ReactVersion = require('ReactVersion'); -ReactDOMInjection.inject(); +require('ReactDOMInjection'); module.exports = { renderToStream: ReactDOMNodeStreamRenderer.renderToStream, diff --git a/src/renderers/dom/ReactDOMServerEntry.js b/src/renderers/dom/ReactDOMServerEntry.js index db0f9dfbd6..48880041dc 100644 --- a/src/renderers/dom/ReactDOMServerEntry.js +++ b/src/renderers/dom/ReactDOMServerEntry.js @@ -11,11 +11,10 @@ 'use strict'; -var ReactDOMInjection = require('ReactDOMInjection'); var ReactDOMStringRenderer = require('ReactDOMStringRenderer'); var ReactVersion = require('ReactVersion'); -ReactDOMInjection.inject(); +require('ReactDOMInjection'); module.exports = { renderToString: ReactDOMStringRenderer.renderToString, diff --git a/src/renderers/dom/ReactDOMServerStackEntry.js b/src/renderers/dom/ReactDOMServerStackEntry.js index 758c6f214c..67ca724da8 100644 --- a/src/renderers/dom/ReactDOMServerStackEntry.js +++ b/src/renderers/dom/ReactDOMServerStackEntry.js @@ -11,13 +11,11 @@ 'use strict'; -var ReactDOMInjection = require('ReactDOMInjection'); -var ReactDOMStackInjection = require('ReactDOMStackInjection'); var ReactServerRendering = require('ReactServerRendering'); var ReactVersion = require('ReactVersion'); -ReactDOMInjection.inject(); -ReactDOMStackInjection.inject(); +require('ReactDOMInjection'); +require('ReactDOMStackInjection'); var ReactDOMServerStack = { renderToString: ReactServerRendering.renderToString, diff --git a/src/renderers/dom/ReactDOMStackEntry.js b/src/renderers/dom/ReactDOMStackEntry.js index 5441f5b353..1c8b8447ea 100644 --- a/src/renderers/dom/ReactDOMStackEntry.js +++ b/src/renderers/dom/ReactDOMStackEntry.js @@ -14,9 +14,6 @@ 'use strict'; var ReactDOMComponentTree = require('ReactDOMComponentTree'); -var ReactDOMInjection = require('ReactDOMInjection'); -var ReactDOMClientInjection = require('ReactDOMClientInjection'); -var ReactDOMStackInjection = require('ReactDOMStackInjection'); var ReactGenericBatching = require('ReactGenericBatching'); var ReactMount = require('ReactMount'); var ReactUpdates = require('ReactUpdates'); @@ -27,9 +24,9 @@ var findDOMNode = require('findDOMNode'); var getHostComponentFromComposite = require('getHostComponentFromComposite'); var warning = require('fbjs/lib/warning'); -ReactDOMInjection.inject(); -ReactDOMClientInjection.inject(); -ReactDOMStackInjection.inject(); +require('ReactDOMInjection'); +require('ReactDOMClientInjection'); +require('ReactDOMStackInjection'); var ReactDOMStack = { findDOMNode: findDOMNode, diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js index 0adca42fdd..0e42bf6899 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js +++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js @@ -22,8 +22,6 @@ var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMFrameScheduling = require('ReactDOMFrameScheduling'); -var ReactDOMClientInjection = require('ReactDOMClientInjection'); -var ReactDOMInjection = require('ReactDOMInjection'); var ReactGenericBatching = require('ReactGenericBatching'); var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactInputSelection = require('ReactInputSelection'); @@ -65,8 +63,8 @@ if (__DEV__) { var {updatedAncestorInfo} = validateDOMNesting; } -ReactDOMClientInjection.inject(); -ReactDOMInjection.inject(); +require('ReactDOMClientInjection'); +require('ReactDOMInjection'); ReactControlledComponent.injection.injectFiberControlledHostComponent( ReactDOMFiberComponent, ); diff --git a/src/renderers/dom/shared/ReactDOMClientInjection.js b/src/renderers/dom/shared/ReactDOMClientInjection.js index 2e70cca0b1..dc929517f4 100644 --- a/src/renderers/dom/shared/ReactDOMClientInjection.js +++ b/src/renderers/dom/shared/ReactDOMClientInjection.js @@ -23,40 +23,24 @@ var ReactDOMEventListener = require('ReactDOMEventListener'); var SelectEventPlugin = require('SelectEventPlugin'); var SimpleEventPlugin = require('SimpleEventPlugin'); -var alreadyInjected = false; +ReactDOMEventListener.setHandleTopLevel( + ReactBrowserEventEmitter.handleTopLevel, +); -function inject() { - if (alreadyInjected) { - // TODO: This is currently true because these injections are shared between - // the client and the server package. They should be built independently - // and not share any injection state. Then this problem will be solved. - return; - } - alreadyInjected = true; +/** + * Inject modules for resolving DOM hierarchy and plugin ordering. + */ +EventPluginHub.injection.injectEventPluginOrder(DOMEventPluginOrder); +EventPluginUtils.injection.injectComponentTree(ReactDOMComponentTree); - ReactDOMEventListener.setHandleTopLevel( - ReactBrowserEventEmitter.handleTopLevel, - ); - - /** - * Inject modules for resolving DOM hierarchy and plugin ordering. - */ - EventPluginHub.injection.injectEventPluginOrder(DOMEventPluginOrder); - EventPluginUtils.injection.injectComponentTree(ReactDOMComponentTree); - - /** - * Some important event plugins included by default (without having to require - * them). - */ - EventPluginHub.injection.injectEventPluginsByName({ - SimpleEventPlugin: SimpleEventPlugin, - EnterLeaveEventPlugin: EnterLeaveEventPlugin, - ChangeEventPlugin: ChangeEventPlugin, - SelectEventPlugin: SelectEventPlugin, - BeforeInputEventPlugin: BeforeInputEventPlugin, - }); -} - -module.exports = { - inject: inject, -}; +/** + * Some important event plugins included by default (without having to require + * them). + */ +EventPluginHub.injection.injectEventPluginsByName({ + SimpleEventPlugin: SimpleEventPlugin, + EnterLeaveEventPlugin: EnterLeaveEventPlugin, + ChangeEventPlugin: ChangeEventPlugin, + SelectEventPlugin: SelectEventPlugin, + BeforeInputEventPlugin: BeforeInputEventPlugin, +}); diff --git a/src/renderers/dom/shared/ReactDOMInjection.js b/src/renderers/dom/shared/ReactDOMInjection.js index 5cd042cfce..e507423f6a 100644 --- a/src/renderers/dom/shared/ReactDOMInjection.js +++ b/src/renderers/dom/shared/ReactDOMInjection.js @@ -16,22 +16,6 @@ var DOMProperty = require('DOMProperty'); var HTMLDOMPropertyConfig = require('HTMLDOMPropertyConfig'); var SVGDOMPropertyConfig = require('SVGDOMPropertyConfig'); -var alreadyInjected = false; - -function inject() { - if (alreadyInjected) { - // TODO: This is currently true because these injections are shared between - // the client and the server package. They should be built independently - // and not share any injection state. Then this problem will be solved. - return; - } - alreadyInjected = true; - - DOMProperty.injection.injectDOMPropertyConfig(ARIADOMPropertyConfig); - DOMProperty.injection.injectDOMPropertyConfig(HTMLDOMPropertyConfig); - DOMProperty.injection.injectDOMPropertyConfig(SVGDOMPropertyConfig); -} - -module.exports = { - inject: inject, -}; +DOMProperty.injection.injectDOMPropertyConfig(ARIADOMPropertyConfig); +DOMProperty.injection.injectDOMPropertyConfig(HTMLDOMPropertyConfig); +DOMProperty.injection.injectDOMPropertyConfig(SVGDOMPropertyConfig); diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index c0607871e5..954780de8f 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -18,8 +18,7 @@ describe('DOMPropertyOperations', () => { beforeEach(() => { jest.resetModules(); - var ReactDOMInjection = require('ReactDOMInjection'); - ReactDOMInjection.inject(); + require('ReactDOMInjection'); // TODO: can we express this test with only public API? DOMPropertyOperations = require('DOMPropertyOperations'); diff --git a/src/renderers/dom/stack/client/ReactDOMStackInjection.js b/src/renderers/dom/stack/client/ReactDOMStackInjection.js index 5a0046976b..d6a9bcebb9 100644 --- a/src/renderers/dom/stack/client/ReactDOMStackInjection.js +++ b/src/renderers/dom/stack/client/ReactDOMStackInjection.js @@ -27,44 +27,28 @@ var ReactUpdates = require('ReactUpdates'); var findDOMNode = require('findDOMNode'); var getHostComponentFromComposite = require('getHostComponentFromComposite'); -var alreadyInjected = false; +ReactGenericBatching.injection.injectStackBatchedUpdates( + ReactUpdates.batchedUpdates, +); -function inject() { - if (alreadyInjected) { - // TODO: This is currently true because these injections are shared between - // the client and the server package. They should be built independently - // and not share any injection state. Then this problem will be solved. - return; - } - alreadyInjected = true; +ReactHostComponent.injection.injectGenericComponentClass(ReactDOMComponent); - ReactGenericBatching.injection.injectStackBatchedUpdates( - ReactUpdates.batchedUpdates, - ); +ReactHostComponent.injection.injectTextComponentClass(ReactDOMTextComponent); - ReactHostComponent.injection.injectGenericComponentClass(ReactDOMComponent); +ReactEmptyComponent.injection.injectEmptyComponentFactory(function( + instantiate, +) { + return new ReactDOMEmptyComponent(instantiate); +}); - ReactHostComponent.injection.injectTextComponentClass(ReactDOMTextComponent); +ReactUpdates.injection.injectReconcileTransaction(ReactReconcileTransaction); +ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy); - ReactEmptyComponent.injection.injectEmptyComponentFactory(function( - instantiate, - ) { - return new ReactDOMEmptyComponent(instantiate); - }); +ReactComponentEnvironment.injection.injectEnvironment( + ReactComponentBrowserEnvironment, +); - ReactUpdates.injection.injectReconcileTransaction(ReactReconcileTransaction); - ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy); - - ReactComponentEnvironment.injection.injectEnvironment( - ReactComponentBrowserEnvironment, - ); - - findDOMNode._injectStack(function(inst) { - inst = getHostComponentFromComposite(inst); - return inst ? ReactDOMComponentTree.getNodeFromInstance(inst) : null; - }); -} - -module.exports = { - inject: inject, -}; +findDOMNode._injectStack(function(inst) { + inst = getHostComponentFromComposite(inst); + return inst ? ReactDOMComponentTree.getNodeFromInstance(inst) : null; +}); diff --git a/src/renderers/native/ReactNativeFiberEntry.js b/src/renderers/native/ReactNativeFiberEntry.js index 2c226e5f43..57c48bc168 100644 --- a/src/renderers/native/ReactNativeFiberEntry.js +++ b/src/renderers/native/ReactNativeFiberEntry.js @@ -15,7 +15,6 @@ const ReactFiberErrorLogger = require('ReactFiberErrorLogger'); const ReactGenericBatching = require('ReactGenericBatching'); const ReactNativeFiberErrorDialog = require('ReactNativeFiberErrorDialog'); -const ReactNativeInjection = require('ReactNativeInjection'); const ReactPortal = require('ReactPortal'); const ReactNativeComponentTree = require('ReactNativeComponentTree'); const ReactNativeFiberRenderer = require('ReactNativeFiberRenderer'); @@ -30,7 +29,7 @@ const {injectInternals} = require('ReactFiberDevToolsHook'); import type {ReactNativeType} from 'ReactNativeTypes'; import type {ReactNodeList} from 'ReactTypes'; -ReactNativeInjection.inject(); +require('ReactNativeInjection'); ReactGenericBatching.injection.injectFiberBatchedUpdates( ReactNativeFiberRenderer.batchedUpdates, diff --git a/src/renderers/native/ReactNativeInjection.js b/src/renderers/native/ReactNativeInjection.js index 116948d7bd..b94c4921cb 100644 --- a/src/renderers/native/ReactNativeInjection.js +++ b/src/renderers/native/ReactNativeInjection.js @@ -29,32 +29,26 @@ var ReactNativeEventPluginOrder = require('ReactNativeEventPluginOrder'); var ReactNativeGlobalResponderHandler = require('ReactNativeGlobalResponderHandler'); var ResponderEventPlugin = require('ResponderEventPlugin'); -function inject() { - /** - * Register the event emitter with the native bridge - */ - RCTEventEmitter.register(ReactNativeEventEmitter); +/** + * Register the event emitter with the native bridge + */ +RCTEventEmitter.register(ReactNativeEventEmitter); - /** - * Inject module for resolving DOM hierarchy and plugin ordering. - */ - EventPluginHub.injection.injectEventPluginOrder(ReactNativeEventPluginOrder); - EventPluginUtils.injection.injectComponentTree(ReactNativeComponentTree); +/** + * Inject module for resolving DOM hierarchy and plugin ordering. + */ +EventPluginHub.injection.injectEventPluginOrder(ReactNativeEventPluginOrder); +EventPluginUtils.injection.injectComponentTree(ReactNativeComponentTree); - ResponderEventPlugin.injection.injectGlobalResponderHandler( - ReactNativeGlobalResponderHandler, - ); +ResponderEventPlugin.injection.injectGlobalResponderHandler( + ReactNativeGlobalResponderHandler, +); - /** - * Some important event plugins included by default (without having to require - * them). - */ - EventPluginHub.injection.injectEventPluginsByName({ - ResponderEventPlugin: ResponderEventPlugin, - ReactNativeBridgeEventPlugin: ReactNativeBridgeEventPlugin, - }); -} - -module.exports = { - inject: inject, -}; +/** + * Some important event plugins included by default (without having to require + * them). + */ +EventPluginHub.injection.injectEventPluginsByName({ + ResponderEventPlugin: ResponderEventPlugin, + ReactNativeBridgeEventPlugin: ReactNativeBridgeEventPlugin, +}); diff --git a/src/renderers/native/ReactNativeStackEntry.js b/src/renderers/native/ReactNativeStackEntry.js index f9a81d41c5..59a71786b4 100644 --- a/src/renderers/native/ReactNativeStackEntry.js +++ b/src/renderers/native/ReactNativeStackEntry.js @@ -12,9 +12,7 @@ 'use strict'; var ReactNativeComponentTree = require('ReactNativeComponentTree'); -var ReactNativeInjection = require('ReactNativeInjection'); var ReactNativeMount = require('ReactNativeMount'); -var ReactNativeStackInjection = require('ReactNativeStackInjection'); var ReactUpdates = require('ReactUpdates'); var ReactNativeStackInspector = require('ReactNativeStackInspector'); @@ -22,8 +20,8 @@ var findNumericNodeHandle = require('findNumericNodeHandleStack'); import type {ReactNativeType} from 'ReactNativeTypes'; -ReactNativeInjection.inject(); -ReactNativeStackInjection.inject(); +require('ReactNativeInjection'); +require('ReactNativeStackInjection'); var render = function( element: ReactElement, diff --git a/src/renderers/native/ReactNativeStackInjection.js b/src/renderers/native/ReactNativeStackInjection.js index 0b8d13d9cf..1bafd389a4 100644 --- a/src/renderers/native/ReactNativeStackInjection.js +++ b/src/renderers/native/ReactNativeStackInjection.js @@ -32,48 +32,40 @@ var ReactUpdates = require('ReactUpdates'); var invariant = require('fbjs/lib/invariant'); -function inject() { - ReactGenericBatching.injection.injectStackBatchedUpdates( - ReactUpdates.batchedUpdates, +ReactGenericBatching.injection.injectStackBatchedUpdates( + ReactUpdates.batchedUpdates, +); + +ReactUpdates.injection.injectReconcileTransaction( + ReactNativeComponentEnvironment.ReactReconcileTransaction, +); + +ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy); + +ReactComponentEnvironment.injection.injectEnvironment( + ReactNativeComponentEnvironment, +); + +var EmptyComponent = instantiate => { + // Can't import View at the top because it depends on React to make its composite + var View = require('View'); + return new ReactSimpleEmptyComponent( + React.createElement(View, { + collapsable: true, + style: {position: 'absolute'}, + }), + instantiate, ); - - ReactUpdates.injection.injectReconcileTransaction( - ReactNativeComponentEnvironment.ReactReconcileTransaction, - ); - - ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy); - - ReactComponentEnvironment.injection.injectEnvironment( - ReactNativeComponentEnvironment, - ); - - var EmptyComponent = instantiate => { - // Can't import View at the top because it depends on React to make its composite - var View = require('View'); - return new ReactSimpleEmptyComponent( - React.createElement(View, { - collapsable: true, - style: {position: 'absolute'}, - }), - instantiate, - ); - }; - - ReactEmptyComponent.injection.injectEmptyComponentFactory(EmptyComponent); - - ReactHostComponent.injection.injectTextComponentClass( - ReactNativeTextComponent, - ); - ReactHostComponent.injection.injectGenericComponentClass(function(tag) { - // Show a nicer error message for non-function tags - var info = ''; - if (typeof tag === 'string' && /^[a-z]/.test(tag)) { - info += ' Each component name should start with an uppercase letter.'; - } - invariant(false, 'Expected a component class, got %s.%s', tag, info); - }); -} - -module.exports = { - inject: inject, }; + +ReactEmptyComponent.injection.injectEmptyComponentFactory(EmptyComponent); + +ReactHostComponent.injection.injectTextComponentClass(ReactNativeTextComponent); +ReactHostComponent.injection.injectGenericComponentClass(function(tag) { + // Show a nicer error message for non-function tags + var info = ''; + if (typeof tag === 'string' && /^[a-z]/.test(tag)) { + info += ' Each component name should start with an uppercase letter.'; + } + invariant(false, 'Expected a component class, got %s.%s', tag, info); +});