From b35acc951611befca61424edb815744bce090aea Mon Sep 17 00:00:00 2001 From: Alexander Bettadapur Date: Sun, 7 Apr 2019 11:30:43 -0700 Subject: [PATCH 1/5] Fix potential issue with ReactRedux 7 --- .../src/DynamicModuleLoader.tsx | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx index e67a9cd..3331695 100644 --- a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx +++ b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx @@ -101,6 +101,7 @@ class DynamicModuleLoaderImpl extends React.Component< private _providerInitializationNeeded: boolean = false; private _store: IModuleStore; private _getLatestState: boolean; + private _memoizedRRContext: any; constructor(props: IDynamicModuleLoaderImplProps) { super(props); @@ -142,10 +143,23 @@ class DynamicModuleLoaderImpl extends React.Component< const { store } = this.props; // store.getState is important here as we don't want to use storeState from the provided context return ( - - {this._renderChildren()} - + + {rrContext => { + if (rrContext !== this._memoizedRRContext) { + this._memoizedRRContext = { + ...rrContext, + storeState: store.getState(), + }; + } + + return ( + + {this._renderChildren()} + + ); + }} + ); }; From db512cc1fd755325f56dc7218a3f8ffc6c47fcef Mon Sep 17 00:00:00 2001 From: Alexander Bettadapur Date: Sun, 7 Apr 2019 12:01:07 -0700 Subject: [PATCH 2/5] Remove support for RR5 --- .../redux-dynamic-modules-react/package.json | 2 +- .../src/DynamicModuleLoader.tsx | 187 +++++++----------- 2 files changed, 71 insertions(+), 118 deletions(-) diff --git a/packages/redux-dynamic-modules-react/package.json b/packages/redux-dynamic-modules-react/package.json index 4343772..36ef6e8 100644 --- a/packages/redux-dynamic-modules-react/package.json +++ b/packages/redux-dynamic-modules-react/package.json @@ -44,7 +44,7 @@ }, "peerDependencies": { "react": ">= 15.0.0", - "react-redux": ">= 5.0.0", + "react-redux": ">= 6.0.0", "redux": ">= 3.0.0" }, "dependencies": { diff --git a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx index 3331695..530437d 100644 --- a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx +++ b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx @@ -1,6 +1,5 @@ -import * as PropTypes from "prop-types"; import * as React from "react"; -//@ts-ignore +//@ts-ignore // ReactReduxContext is not officially exported import { Provider, ReactReduxContext } from "react-redux"; import { @@ -19,10 +18,6 @@ export interface IDynamicModuleLoaderProps { createStore?: () => IModuleStore; } -export interface IDynamicModuleLoaderContext { - store: IModuleStore; -} - /** * The DynamicModuleLoader adds a way to register a module on mount * When this component is initialized, the reducer and saga from the module passed as props will be registered with the system @@ -31,82 +26,44 @@ export interface IDynamicModuleLoaderContext { export class DynamicModuleLoader extends React.Component< IDynamicModuleLoaderProps > { - // @ts-ignore - private static contextTypes = { - store: PropTypes.object, - }; - - constructor( - props: IDynamicModuleLoaderProps, - context: IDynamicModuleLoaderContext - ) { - super(props, context); - } - - /** - * Render a Redux provider - */ - public render(): React.ReactNode { - if (ReactReduxContext) { - return ( - - {context => { - return ( - - {this.props.children} - - ); - }} - - ); - } else { - return ( - - {this.props.children} - - ); - } + public render() { + return ( + + {reactReduxContext => ( + + )} + + ); } } -interface IDynamicModuleLoaderImplProps { - /** Modules that need to be dynamically registerd */ - modules: IModuleTuple; - - store: IModuleStore; - - strictMode: boolean; - - createStore?: () => IModuleStore; +export interface IDynamicModuleLoaderImplProps + extends IDynamicModuleLoaderProps { + reactReduxContext?: { store: IModuleStore }; } -interface IDynamicModuleLoaderImplState { +export interface IDynamicModuleLoaderImplState { readyToRender: boolean; } -class DynamicModuleLoaderImpl extends React.Component< +export class DynamicModuleLoaderImpl extends React.Component< IDynamicModuleLoaderImplProps, IDynamicModuleLoaderImplState > { private _addedModules?: IDynamicallyAddedModule; private _providerInitializationNeeded: boolean = false; private _store: IModuleStore; - private _getLatestState: boolean; private _memoizedRRContext: any; constructor(props: IDynamicModuleLoaderImplProps) { super(props); - this._store = this.props.store; + this._store = props.reactReduxContext + ? props.reactReduxContext.store + : undefined; // We are not in strict mode, let's add the modules ASAP if (!this.props.strictMode) { @@ -119,73 +76,69 @@ class DynamicModuleLoaderImpl extends React.Component< } } - private _addModules(): void { - const { createStore, modules } = this.props; - if (!this._store) { - if (createStore) { - this._store = createStore(); - this._providerInitializationNeeded = true; - } else { - throw new Error( - "Store could not be resolved from React context" + public render(): React.ReactNode { + if (this.state.readyToRender) { + if (this._providerInitializationNeeded) { + return ( + + {/* We just rendered the provider, so now we need to render + DML again. This one will add the modules */} + + ); } - } else { - // We will add modules dynamically and due to github issue https://github.com/Microsoft/redux-dynamic-modules/issues/27#issuecomment-464905893 - // The very first render will not get latest state, to fix that we will need to get latest state from store directly on first render - this._getLatestState = ReactReduxContext; + + return this._renderLoader(); } - this._addedModules = this._store.addModules(modules); + return null; } - private _renderWithReactReduxContext = () => { - const { store } = this.props; - // store.getState is important here as we don't want to use storeState from the provided context - return ( - - {rrContext => { - if (rrContext !== this._memoizedRRContext) { - this._memoizedRRContext = { - ...rrContext, - storeState: store.getState(), - }; - } - - return ( - - {this._renderChildren()} - - ); - }} - - ); - }; + /** + * Render a Redux provider + */ + private _renderLoader(): React.ReactNode { + if (this.props.reactReduxContext == null) { + const message = + "Tried to render DynamicModuleLoader, but no ReactReduxContext was provided"; + console.error(message); - private _renderChildren = () => { - if (this.props.children && typeof this.props.children === "function") { - return this.props.children(); + throw new Error(message); } - return this.props.children; - }; + // Memoize the context if it has changed upstream + if (this.props.reactReduxContext !== this._memoizedRRContext) { + this._memoizedRRContext = { + ...this.props.reactReduxContext, + storeState: this.props.reactReduxContext.store.getState(), + }; + } - public render(): React.ReactNode { - if (this.state.readyToRender) { - if (this._providerInitializationNeeded) { - return ( - - {this._renderChildren()} - + return ( + + {this.props.children && + typeof this.props.children === "function" + ? this.props.children() + : this.props.children} + + ); + } + + private _addModules(): void { + const { createStore, modules } = this.props; + + if (!this._store) { + if (createStore) { + this._store = createStore(); + this._providerInitializationNeeded = true; + } else { + throw new Error( + "Store could not be resolved from React context" ); - } else if (!this._getLatestState) { - return this._renderChildren(); } - - return this._renderWithReactReduxContext(); + } else { + this._addedModules = this._store.addModules(modules); } - return null; } public componentDidMount() { From 2fad8530b020484c883072ceda6fbecb7987513d Mon Sep 17 00:00:00 2001 From: Alexander Bettadapur Date: Sun, 7 Apr 2019 12:05:26 -0700 Subject: [PATCH 3/5] Remove exports --- .../src/DynamicModuleLoader.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx index 530437d..956fa7e 100644 --- a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx +++ b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx @@ -40,16 +40,15 @@ export class DynamicModuleLoader extends React.Component< } } -export interface IDynamicModuleLoaderImplProps - extends IDynamicModuleLoaderProps { +interface IDynamicModuleLoaderImplProps extends IDynamicModuleLoaderProps { reactReduxContext?: { store: IModuleStore }; } -export interface IDynamicModuleLoaderImplState { +interface IDynamicModuleLoaderImplState { readyToRender: boolean; } -export class DynamicModuleLoaderImpl extends React.Component< +class DynamicModuleLoaderImpl extends React.Component< IDynamicModuleLoaderImplProps, IDynamicModuleLoaderImplState > { @@ -128,6 +127,7 @@ export class DynamicModuleLoaderImpl extends React.Component< const { createStore, modules } = this.props; if (!this._store) { + // If we need to create a store, do that here. We will skip adding the modules and render DML again if (createStore) { this._store = createStore(); this._providerInitializationNeeded = true; @@ -137,6 +137,7 @@ export class DynamicModuleLoaderImpl extends React.Component< ); } } else { + // Add the modules here this._addedModules = this._store.addModules(modules); } } From 4cb3321409d37a8d02de1940dd843d58c4309fe3 Mon Sep 17 00:00:00 2001 From: Alexander Bettadapur Date: Mon, 29 Apr 2019 09:58:09 -0700 Subject: [PATCH 4/5] CR --- .../src/DynamicModuleLoader.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx index 956fa7e..1b677fe 100644 --- a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx +++ b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx @@ -52,10 +52,14 @@ class DynamicModuleLoaderImpl extends React.Component< IDynamicModuleLoaderImplProps, IDynamicModuleLoaderImplState > { + /** The modules that were added from this loader */ private _addedModules?: IDynamicallyAddedModule; + /** Flag that indicates we need to create a store/provider because a parent store was not provided */ private _providerInitializationNeeded: boolean = false; + /** The module store, derived from context */ private _store: IModuleStore; - private _memoizedRRContext: any; + /** The react redux context, saved */ + private _memoizedReactReduxContext: any; constructor(props: IDynamicModuleLoaderImplProps) { super(props); @@ -106,15 +110,17 @@ class DynamicModuleLoaderImpl extends React.Component< } // Memoize the context if it has changed upstream - if (this.props.reactReduxContext !== this._memoizedRRContext) { - this._memoizedRRContext = { + // If the context has not changed, we want to use the same object reference so that + // downstream consumers do not update needlessly + if (this.props.reactReduxContext !== this._memoizedReactReduxContext) { + this._memoizedReactReduxContext = { ...this.props.reactReduxContext, storeState: this.props.reactReduxContext.store.getState(), }; } return ( - + {this.props.children && typeof this.props.children === "function" ? this.props.children() From b75fbd90443c8b37913b140acc5a6e01f5fedf81 Mon Sep 17 00:00:00 2001 From: Alexander Bettadapur Date: Mon, 29 Apr 2019 22:09:14 -0700 Subject: [PATCH 5/5] Add more doc comments: --- .../src/DynamicModuleLoader.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx index 1b677fe..cc4ffa2 100644 --- a/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx +++ b/packages/redux-dynamic-modules-react/src/DynamicModuleLoader.tsx @@ -12,9 +12,15 @@ export interface IDynamicModuleLoaderProps { /** Modules that need to be dynamically registerd */ modules: IModuleTuple; + /** + * Set this flag to indicate that this component is being rendered in 'Strict Mode' + * React 'StrictMode' does not allow constructor side-effects, so we defer adding modules to componentDidMount + * when this flag is set. + * This has the effect of adding a second render. + */ strictMode?: boolean; - /** Optional callback which returns a store instance. This would be called if no store could be loaded from the context. */ + /** Optional callback which returns a store instance. This would be called if no store could be loaded from th e context. */ createStore?: () => IModuleStore; } @@ -41,10 +47,15 @@ export class DynamicModuleLoader extends React.Component< } interface IDynamicModuleLoaderImplProps extends IDynamicModuleLoaderProps { + /** The react-redux context passed from the component */ reactReduxContext?: { store: IModuleStore }; } interface IDynamicModuleLoaderImplState { + /** Is the DML component ready to render. + * If strictMode is set to false, this will be set to true initially + * If strict mode is set to true, this will be set after the first render completes + */ readyToRender: boolean; }