Avoid calling component generators on registerComponent (#4286)

* Avoid calling component generators on registerComponent

* fix unit tests

* remove console.log

* fix coverage

* fix coverage

Author: yogevbd

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;
   }
 }