Avoid calling component generators on registerComponent (#4286)

* Avoid calling component generators on registerComponent

* fix unit tests

* remove console.log

* fix coverage

* fix coverage

docs/api/Store.md

@@ -16,17 +16,17 @@
 
 ---
 
-## setOriginalComponentClassForName
+## setComponentClassForName
 
-`setOriginalComponentClassForName(componentName: string, ComponentClass: any): void`
+`setComponentClassForName(componentName: string, ComponentClass: any): void`
 
 [source](https://github.com/wix/react-native-navigation/blob/v2/lib/src/components/Store.ts#L15)
 
 ---
 
-## getOriginalComponentClassForName
+## getComponentClassForName
 
-`getOriginalComponentClassForName(componentName: string): any`
+`getComponentClassForName(componentName: string): any`
 
 [source](https://github.com/wix/react-native-navigation/blob/v2/lib/src/components/Store.ts#L19)
 

integration/redux/Redux.test.js

@@ -41,7 +41,7 @@ describe('redux support', () => {
         );
       }
     };
-    const CompFromNavigation = Navigation.registerComponent('ComponentName', () => HOC);
+    const CompFromNavigation = Navigation.registerComponent('ComponentName', () => HOC)();
 
     const tree = renderer.create(<CompFromNavigation componentId='componentId' renderCountIncrement={renderCountIncrement}/>);
     expect(tree.toJSON().children).toEqual(['no name']);

integration/remx/remx.test.js

@@ -48,7 +48,7 @@ describe('remx support', () => {
   it('support for static members in connected components', () => {
     expect(MyConnectedComponent.options).toEqual({ title: 'MyComponent' });
 
-    const registeredComponentClass = Navigation.registerComponent('MyComponentName', () => MyConnectedComponent);
+    const registeredComponentClass = Navigation.registerComponent('MyComponentName', () => MyConnectedComponent)();
     expect(registeredComponentClass.options).toEqual({ title: 'MyComponent' });
   });
 });

lib/src/Navigation.ts

@@ -11,11 +11,11 @@ import { ComponentProvider } from 'react-native';
 import { Element } from './adapters/Element';
 import { CommandsObserver } from './events/CommandsObserver';
 import { Constants } from './adapters/Constants';
-import { ComponentType } from 'react';
 import { ComponentEventsObserver } from './events/ComponentEventsObserver';
 import { TouchablePreview } from './adapters/TouchablePreview';
 import { LayoutRoot, Layout } from './interfaces/Layout';
 import { Options } from './interfaces/Options';
+import { ComponentWrapper } from './components/ComponentWrapper';
 
 export class Navigation {
   public readonly Element: React.ComponentType<{ elementId: any; resizeMode?: any; }>;
@@ -31,15 +31,17 @@ export class Navigation {
   private readonly eventsRegistry: EventsRegistry;
   private readonly commandsObserver: CommandsObserver;
   private readonly componentEventsObserver: ComponentEventsObserver;
+  private readonly componentWrapper: typeof ComponentWrapper;
 
   constructor() {
     this.Element = Element;
     this.TouchablePreview = TouchablePreview;
     this.store = new Store();
+    this.componentWrapper = ComponentWrapper;
     this.nativeEventsReceiver = new NativeEventsReceiver();
     this.uniqueIdProvider = new UniqueIdProvider();
     this.componentEventsObserver = new ComponentEventsObserver(this.nativeEventsReceiver);
-    this.componentRegistry = new ComponentRegistry(this.store, this.componentEventsObserver);
+    this.componentRegistry = new ComponentRegistry(this.store, this.componentEventsObserver, this.componentWrapper);
     this.layoutTreeParser = new LayoutTreeParser();
     this.layoutTreeCrawler = new LayoutTreeCrawler(this.uniqueIdProvider, this.store);
     this.nativeCommandsSender = new NativeCommandsSender();
@@ -54,7 +56,7 @@ export class Navigation {
    * Every navigation component in your app must be registered with a unique name.
    * The component itself is a traditional React component extending React.Component.
    */
-  public registerComponent(componentName: string, getComponentClassFunc: ComponentProvider): ComponentType<any> {
+  public registerComponent(componentName: string, getComponentClassFunc: ComponentProvider): ComponentProvider {
     return this.componentRegistry.registerComponent(componentName, getComponentClassFunc);
   }
 
@@ -67,7 +69,7 @@ export class Navigation {
     getComponentClassFunc: ComponentProvider,
     ReduxProvider: any,
     reduxStore: any
-  ): ComponentType<any> {
+  ): ComponentProvider {
     return this.componentRegistry.registerComponent(componentName, getComponentClassFunc, ReduxProvider, reduxStore);
   }
 

lib/src/commands/Commands.test.ts

@@ -254,6 +254,15 @@ describe('Commands', () => {
         children: []
       });
     });
+
+    it('calls component generator once', async () => {
+      const generator = jest.fn(() => {
+        return {};
+      });
+      store.setComponentClassForName('theComponentName', generator);
+      await uut.push('theComponentId', { component: { name: 'theComponentName' } });
+      expect(generator).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('pop', () => {

lib/src/commands/LayoutTreeCrawler.test.ts

@@ -62,11 +62,44 @@ describe('LayoutTreeCrawler', () => {
     };
 
     const node: any = { type: LayoutType.Component, data: { name: 'theComponentName' } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
     uut.crawl(node);
     expect(node.data.options).toEqual(theStyle);
   });
 
+  it('Components: crawl does not cache options', () => {
+    const optionsWithTitle = (title) => {
+      return {
+        topBar: {
+          title: {
+            text: title
+          }
+        }
+      }
+    };
+
+    const MyComponent = class {
+      static options(props) {
+        return {
+          topBar: {
+            title: {
+              text: props.title
+            }
+          }
+        };
+      }
+    };
+
+    const node: any = { type: LayoutType.Component, data: { name: 'theComponentName', passProps: { title: 'title' } } };
+    store.setComponentClassForName('theComponentName', () => MyComponent);
+    uut.crawl(node);
+    expect(node.data.options).toEqual(optionsWithTitle('title'));
+
+    const node2: any = { type: LayoutType.Component, data: { name: 'theComponentName' } };
+    uut.crawl(node2);
+    expect(node2.data.options).toEqual(optionsWithTitle(undefined));
+  });
+
   it('Components: passes passProps to the static options function to be used by the user', () => {
     const MyComponent = class {
       static options(passProps) {
@@ -75,7 +108,7 @@ describe('LayoutTreeCrawler', () => {
     };
 
     const node: any = { type: LayoutType.Component, data: { name: 'theComponentName', passProps: { bar: { baz: { value: 'hello' } } } } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
     uut.crawl(node);
     expect(node.data.options).toEqual({ foo: 'hello' });
   });
@@ -88,7 +121,7 @@ describe('LayoutTreeCrawler', () => {
     };
 
     const node: any = { type: LayoutType.Component, data: { name: 'theComponentName' } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
     uut.crawl(node);
     expect(node.data.options).toEqual({ foo: {} });
   });
@@ -116,7 +149,7 @@ describe('LayoutTreeCrawler', () => {
     };
 
     const node = { type: LayoutType.Component, data: { name: 'theComponentName', options: passedOptions } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
 
     uut.crawl(node);
 
@@ -139,7 +172,7 @@ describe('LayoutTreeCrawler', () => {
     };
 
     const node: any = { type: LayoutType.Component, data: { name: 'theComponentName' } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
     uut.crawl(node);
     expect(node.data.options).not.toBe(theStyle);
   });
@@ -153,7 +186,7 @@ describe('LayoutTreeCrawler', () => {
     const MyComponent = class { };
 
     const node: any = { type: LayoutType.Component, data: { name: 'theComponentName' } };
-    store.setOriginalComponentClassForName('theComponentName', MyComponent);
+    store.setComponentClassForName('theComponentName', () => MyComponent);
     uut.crawl(node);
     expect(node.data.options).toEqual({});
   });

lib/src/commands/LayoutTreeCrawler.ts

@@ -52,7 +52,7 @@ export class LayoutTreeCrawler {
   }
 
   _applyStaticOptions(node) {
-    const clazz = this.store.getOriginalComponentClassForName(node.data.name) || {};
+    const clazz = this.store.getComponentClassForName(node.data.name) ? this.store.getComponentClassForName(node.data.name)() : {};
     const staticOptions = _.isFunction(clazz.options) ? clazz.options(node.data.passProps || {}) : (_.cloneDeep(clazz.options) || {});
     const passedOptions = node.data.options || {};
     node.data.options = _.merge({}, staticOptions, passedOptions);

lib/src/components/ComponentRegistry.test.tsx

@@ -8,8 +8,10 @@ describe('ComponentRegistry', () => {
   let uut;
   let store;
   let mockRegistry: any;
+  let mockWrapper: any;
 
-  class MyComponent extends React.Component {
+
+  class WrappedComponent extends React.Component {
     render() {
       return (
         <Text>
@@ -23,30 +25,46 @@ describe('ComponentRegistry', () => {
   beforeEach(() => {
     store = new Store();
     mockRegistry = AppRegistry.registerComponent = jest.fn(AppRegistry.registerComponent);
-    uut = new ComponentRegistry(store, {} as any);
+    mockWrapper = jest.mock('./ComponentWrapper');
+    mockWrapper.wrap = () => WrappedComponent;
+    uut = new ComponentRegistry(store, {} as any, mockWrapper);
   });
 
-  it('registers component component by componentName into AppRegistry', () => {
+  it('registers component by componentName into AppRegistry', () => {
     expect(mockRegistry).not.toHaveBeenCalled();
-    const result = uut.registerComponent('example.MyComponent.name', () => MyComponent);
+    const result = uut.registerComponent('example.MyComponent.name', () => {});
     expect(mockRegistry).toHaveBeenCalledTimes(1);
     expect(mockRegistry.mock.calls[0][0]).toEqual('example.MyComponent.name');
-    expect(mockRegistry.mock.calls[0][1]()).toEqual(result);
+    expect(mockRegistry.mock.calls[0][1]()).toEqual(result());
   });
 
-  it('saves the original component into the store', () => {
-    expect(store.getOriginalComponentClassForName('example.MyComponent.name')).toBeUndefined();
-    uut.registerComponent('example.MyComponent.name', () => MyComponent);
-    const Class = store.getOriginalComponentClassForName('example.MyComponent.name');
+  it('saves the wrapper component generator the store', () => {
+    expect(store.getComponentClassForName('example.MyComponent.name')).toBeUndefined();
+    uut.registerComponent('example.MyComponent.name', () => {});
+    const Class = store.getComponentClassForName('example.MyComponent.name');
     expect(Class).not.toBeUndefined();
-    expect(Class).toEqual(MyComponent);
-    expect(Object.getPrototypeOf(Class)).toEqual(React.Component);
+    expect(Class()).toEqual(WrappedComponent);
+    expect(Object.getPrototypeOf(Class())).toEqual(React.Component);
   });
 
   it('resulting in a normal component', () => {
-    uut.registerComponent('example.MyComponent.name', () => MyComponent);
+    uut.registerComponent('example.MyComponent.name', () => {});
     const Component = mockRegistry.mock.calls[0][1]();
     const tree = renderer.create(<Component componentId='123' />);
     expect(tree.toJSON()!.children).toEqual(['Hello, World!']);
   });
+
+  it('should not invoke generator', () => {
+    const generator = jest.fn(() => {});
+    uut.registerComponent('example.MyComponent.name', generator);
+    expect(generator).toHaveBeenCalledTimes(0);
+  });
+
+  it('saves wrapped component to store', () => {
+    jest.spyOn(store, 'setComponentClassForName');
+    const generator = jest.fn(() => {});
+    const componentName = 'example.MyComponent.name';
+    uut.registerComponent(componentName, generator);
+    expect(store.getComponentClassForName(componentName)()).toEqual(WrappedComponent);
+  });
 });

lib/src/components/ComponentRegistry.ts

@@ -1,17 +1,17 @@
 import { AppRegistry, ComponentProvider } from 'react-native';
 import { ComponentWrapper } from './ComponentWrapper';
-import { ComponentType } from 'react';
 import { Store } from './Store';
 import { ComponentEventsObserver } from '../events/ComponentEventsObserver';
 
 export class ComponentRegistry {
-  constructor(private readonly store: Store, private readonly componentEventsObserver: ComponentEventsObserver) { }
+  constructor(private readonly store: Store, private readonly componentEventsObserver: ComponentEventsObserver, private readonly ComponentWrapperClass: typeof ComponentWrapper) { }
 
-  registerComponent(componentName: string, getComponentClassFunc: ComponentProvider, ReduxProvider?: any, reduxStore?: any): ComponentType<any> {
-    const OriginalComponentClass = getComponentClassFunc();
-    const NavigationComponent = ComponentWrapper.wrap(componentName, OriginalComponentClass, this.store, this.componentEventsObserver, ReduxProvider, reduxStore);
-    this.store.setOriginalComponentClassForName(componentName, OriginalComponentClass);
-    AppRegistry.registerComponent(componentName, () => NavigationComponent);
+  registerComponent(componentName: string, getComponentClassFunc: ComponentProvider, ReduxProvider?: any, reduxStore?: any): ComponentProvider {
+    const NavigationComponent = () => {
+      return this.ComponentWrapperClass.wrap(componentName, getComponentClassFunc, this.store, this.componentEventsObserver, ReduxProvider, reduxStore)
+    };
+    this.store.setComponentClassForName(componentName, NavigationComponent);
+    AppRegistry.registerComponent(componentName, NavigationComponent);
     return NavigationComponent;
   }
 }

lib/src/components/ComponentWrapper.test.tsx

@@ -53,7 +53,7 @@ describe('ComponentWrapper', () => {
   });
 
   it('must have componentId as prop', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const orig = console.error;
     console.error = (a) => a;
     expect(() => {
@@ -63,7 +63,7 @@ describe('ComponentWrapper', () => {
   });
 
   it('wraps the component', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     expect(NavigationComponent).not.toBeInstanceOf(MyComponent);
     const tree = renderer.create(<NavigationComponent componentId={'component1'} />);
     expect(tree.toJSON()!.children).toEqual(['Hello, World!']);
@@ -71,14 +71,14 @@ describe('ComponentWrapper', () => {
 
   it('injects props from wrapper into original component', () => {
     const renderCount = jest.fn();
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<NavigationComponent componentId={'component1'} text={'yo'} renderCount={renderCount} />);
     expect(tree.toJSON()!.children).toEqual(['yo']);
     expect(renderCount).toHaveBeenCalledTimes(1);
   });
 
   it('updates props from wrapper into original component on state change', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<TestParent ChildClass={NavigationComponent} />);
     expect(myComponentProps.foo).toEqual(undefined);
     (tree.getInstance() as any).setState({ propsFromState: { foo: 'yo' } });
@@ -87,13 +87,13 @@ describe('ComponentWrapper', () => {
 
   it('pulls props from the store and injects them into the inner component', () => {
     store.setPropsForId('component123', { numberProp: 1, stringProp: 'hello', objectProp: { a: 2 } });
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     renderer.create(<NavigationComponent componentId={'component123'} />);
     expect(myComponentProps).toEqual({ componentId: 'component123', numberProp: 1, stringProp: 'hello', objectProp: { a: 2 } });
   });
 
   it('updates props from store into inner component', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<TestParent ChildClass={NavigationComponent} />);
     store.setPropsForId('component1', { myProp: 'hello' });
     expect(myComponentProps.foo).toEqual(undefined);
@@ -104,7 +104,7 @@ describe('ComponentWrapper', () => {
   });
 
   it('protects id from change', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<TestParent ChildClass={NavigationComponent} />);
     expect(myComponentProps.componentId).toEqual('component1');
     (tree.getInstance() as any).setState({ propsFromState: { id: 'ERROR' } });
@@ -112,7 +112,7 @@ describe('ComponentWrapper', () => {
   });
 
   it('assignes key by id', () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<NavigationComponent componentId={'component1'} />);
     expect(myComponentProps.componentId).toEqual('component1');
     expect((tree.getInstance() as any)._reactInternalInstance.child.key).toEqual('component1');
@@ -120,20 +120,20 @@ describe('ComponentWrapper', () => {
 
   it('cleans props from store on unMount', () => {
     store.setPropsForId('component123', { foo: 'bar' });
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<NavigationComponent componentId={'component123'} />);
     expect(store.getPropsForId('component123')).toEqual({ foo: 'bar' });
     tree.unmount();
     expect(store.getPropsForId('component123')).toEqual({});
   });
 
-  it(`merges static members from wrapped component`, () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver) as any;
+  it(`merges static members from wrapped component when generated`, () => {
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver) as any;
     expect(NavigationComponent.options).toEqual({ title: 'MyComponentTitle' });
   });
 
   it(`calls unmounted on componentEventsObserver`, () => {
-    const NavigationComponent = ComponentWrapper.wrap(componentName, MyComponent, store, componentEventsObserver);
+    const NavigationComponent = ComponentWrapper.wrap(componentName, () => MyComponent, store, componentEventsObserver);
     const tree = renderer.create(<NavigationComponent componentId={'component123'} />);
     verify(mockedComponentEventsObserver.unmounted('component123')).never();
     tree.unmount();
@@ -162,7 +162,7 @@ describe('ComponentWrapper', () => {
     const reduxStore = require('redux').createStore((state = initialState) => state);
 
     it(`wraps the component with a react-redux provider with passed store`, () => {
-      const NavigationComponent = ComponentWrapper.wrap(componentName, ConnectedComp, store, componentEventsObserver, ReduxProvider, reduxStore);
+      const NavigationComponent = ComponentWrapper.wrap(componentName, () => ConnectedComp, store, componentEventsObserver, ReduxProvider, reduxStore);
       const tree = renderer.create(<NavigationComponent componentId={'theCompId'} />);
       expect(tree.toJSON()!.children).toEqual(['it just works']);
       expect((NavigationComponent as any).options).toEqual({ foo: 123 });

lib/src/components/ComponentWrapper.tsx

@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { ComponentProvider } from 'react-native';
 import * as  _ from 'lodash';
 import * as ReactLifecyclesCompat from 'react-lifecycles-compat';
 import { Store } from './Store';
@@ -10,12 +11,13 @@ interface HocProps { componentId: string; }
 export class ComponentWrapper {
   static wrap(
     componentName: string,
-    OriginalComponentClass: React.ComponentType<any>,
+    OriginalComponentGenerator: ComponentProvider,
     store: Store,
     componentEventsObserver: ComponentEventsObserver,
     ReduxProvider?: any,
     reduxStore?: any
   ): React.ComponentClass<any> {
+    const GeneratedComponentClass = OriginalComponentGenerator();
     class WrappedComponent extends React.Component<HocProps, HocState> {
       static getDerivedStateFromProps(nextProps: any, prevState: HocState) {
         return {
@@ -39,7 +41,7 @@ export class ComponentWrapper {
 
       render() {
         return (
-          <OriginalComponentClass
+          <GeneratedComponentClass
             {...this.state.allProps}
             componentId={this.state.componentId}
             key={this.state.componentId}
@@ -55,7 +57,7 @@ export class ComponentWrapper {
     }
 
     ReactLifecyclesCompat.polyfill(WrappedComponent);
-    require('hoist-non-react-statics')(WrappedComponent, OriginalComponentClass);
+    require('hoist-non-react-statics')(WrappedComponent, GeneratedComponentClass);
 
     if (reduxStore && ReduxProvider) {
       return ComponentWrapper.wrapWithRedux(WrappedComponent, ReduxProvider, reduxStore);

lib/src/components/Store.test.ts

@@ -26,8 +26,8 @@ describe('Store', () => {
     const MyComponent = class {
       //
     };
-    uut.setOriginalComponentClassForName('example.mycomponent', MyComponent);
-    expect(uut.getOriginalComponentClassForName('example.mycomponent')).toEqual(MyComponent);
+    uut.setComponentClassForName('example.mycomponent', MyComponent);
+    expect(uut.getComponentClassForName('example.mycomponent')).toEqual(MyComponent);
   });
 
   it('clean by component id', () => {

lib/src/components/Store.ts

@@ -12,14 +12,14 @@ export class Store {
     return _.get(this.propsById, componentId, {});
   }
 
-  setOriginalComponentClassForName(componentName: string, ComponentClass) {
+  setComponentClassForName(componentName: string, ComponentClass) {
     _.set(this.componentsByName, componentName, ComponentClass);
   }
 
-  getOriginalComponentClassForName(componentName: string) {
+  getComponentClassForName(componentName: string) {
     return _.get(this.componentsByName, componentName);
   }
-
+  
   cleanId(id: string) {
     _.unset(this.propsById, id);
   }