feat(router): withRouterIfEnabled - fixed tests, made sure history is treated as optional in affected components

Author: rafalmaksymiuk

src/elements/common/routing/__tests__/withRouterIfEnabled.test.js

@@ -4,62 +4,52 @@ import { MemoryRouter } from 'react-router-dom';
 import { render } from '../../../../test-utils/testing-library';
 import withRouterIfEnabled from '../withRouterIfEnabled';
 
-describe('elements/common/routing/withRouterIfEnabled', () => {
-    const TestComponent = (props: any) => {
-        const { history, location, match, routerDisabled } = props;
-        return (
-            // Expose which props were injected
-            <div
-                data-testid="test-component"
-                data-router-disabled={routerDisabled ? 'true' : undefined}
-                data-has-history={history ? 'true' : undefined}
-                data-has-location={location ? 'true' : undefined}
-                data-has-match={match ? 'true' : undefined}
-            />
-        );
-    };
-    TestComponent.displayName = 'TestComponent';
-
-    const WithRouterIfEnabled = withRouterIfEnabled(TestComponent);
-
-    describe('router enabled (default)', () => {
-        test('injects router props when wrapped in a Router', () => {
-            const { getByTestId } = render(
-                <MemoryRouter initialEntries={[{ pathname: '/foo' }]}>
-                    <WithRouterIfEnabled />
-                </MemoryRouter>,
-            );
-
-            const el = getByTestId('test-component');
-            expect(el).toHaveAttribute('data-has-history', 'true');
-            expect(el).toHaveAttribute('data-has-location', 'true');
-            expect(el).toHaveAttribute('data-has-match', 'true');
-            expect(el).not.toHaveAttribute('data-router-disabled');
-        });
-    });
+const TestComponent = (props: any) => {
+    const { history, location, match, routerDisabled } = props;
+    return (
+        <div
+            data-testid="test-component"
+            data-router-disabled={routerDisabled ? 'true' : undefined}
+            data-has-history={history ? 'true' : undefined}
+            data-has-location={location ? 'true' : undefined}
+            data-has-match={match ? 'true' : undefined}
+        />
+    );
+};
+TestComponent.displayName = 'TestComponent';
+
+const WithRouterIfEnabled = withRouterIfEnabled(TestComponent);
+
+test('injects router props when wrapped in a Router', () => {
+    const { getByTestId } = render(
+        <MemoryRouter initialEntries={[{ pathname: '/foo' }]}> 
+            <WithRouterIfEnabled />
+        </MemoryRouter>,
+    );
+
+    const el = getByTestId('test-component');
+    expect(el).toHaveAttribute('data-has-history', 'true');
+    expect(el).toHaveAttribute('data-has-location', 'true');
+    expect(el).toHaveAttribute('data-has-match', 'true');
+    expect(el).not.toHaveAttribute('data-router-disabled');
+});
 
-    describe('router disabled via prop', () => {
-        test('renders without Router and without router props', () => {
-            const { getByTestId } = render(<WithRouterIfEnabled routerDisabled />);
-            const el = getByTestId('test-component');
-            expect(el).not.toHaveAttribute('data-has-history');
-            expect(el).not.toHaveAttribute('data-has-location');
-            expect(el).not.toHaveAttribute('data-has-match');
-            expect(el).toHaveAttribute('data-router-disabled', 'true');
-        });
-    });
+test('renders without Router and without router props (routerDisabled prop)', () => {
+    const { getByTestId } = render(<WithRouterIfEnabled routerDisabled />);
+    const el = getByTestId('test-component');
+    expect(el).not.toHaveAttribute('data-has-history');
+    expect(el).not.toHaveAttribute('data-has-location');
+    expect(el).not.toHaveAttribute('data-has-match');
+    expect(el).toHaveAttribute('data-router-disabled', 'true');
+});
 
-    describe('router disabled via feature flag', () => {
-        test('renders without Router and without router props when feature is enabled', () => {
-            const { getByTestId } = render(<WithRouterIfEnabled />, {
-                wrapperProps: { features: { routerDisabled: { value: true } } },
-            });
+test('renders without Router and without router props (feature flag)', () => {
+    const features = { routerDisabled: { value: true } };
+    const { getByTestId } = render(<WithRouterIfEnabled features={features} />);
 
-            const el = getByTestId('test-component');
-            expect(el).not.toHaveAttribute('data-has-history');
-            expect(el).not.toHaveAttribute('data-has-location');
-            expect(el).not.toHaveAttribute('data-has-match');
-        });
-    });
+    const el = getByTestId('test-component');
+    expect(el).not.toHaveAttribute('data-has-history');
+    expect(el).not.toHaveAttribute('data-has-location');
+    expect(el).not.toHaveAttribute('data-has-match');
 });
 

src/elements/common/routing/withRouterIfEnabled.js

@@ -4,13 +4,13 @@ import { withRouter } from 'react-router-dom';
 import { isFeatureEnabled } from '../feature-checking';
 
 export default function withRouterIfEnabled(Wrapped: React.ComponentType<any>) {
-    const WrappedWithRouter = withRouter(Wrapped);
 
     const WithRouterIfEnabled = (props: any) => {
         const routerDisabled =
             props?.routerDisabled === true || isFeatureEnabled(props?.features, 'routerDisabled.value');
 
-        const Component = routerDisabled ? Wrapped : WrappedWithRouter;
+        const Component = routerDisabled ? Wrapped : withRouter(Wrapped);
+
         return <Component {...props} />;
     };
 

src/elements/content-sidebar/AddTaskButton.js

@@ -12,7 +12,7 @@ import type { ElementsXhrError } from '../../common/types/api';
 import type { InternalSidebarNavigation, InternalSidebarNavigationHandler } from '../common/types/SidebarNavigation';
 
 type Props = {|
-    history: RouterHistory,
+    history?: RouterHistory,
     internalSidebarNavigation?: InternalSidebarNavigation,
     internalSidebarNavigationHandler?: InternalSidebarNavigationHandler,
     isDisabled: boolean,
@@ -55,7 +55,7 @@ class AddTaskButton extends React.Component<Props, State> {
                 },
                 true,
             );
-        } else {
+        } else if (history) {
             history.replace({ state: { open: true } });
         }
 

src/elements/content-sidebar/__tests__/__snapshots__/ActivitySidebar.test.js.snap

@@ -4,7 +4,7 @@ exports[`elements/content-sidebar/ActivitySidebar render() should render the act
 <SidebarContent
   actions={
     <React.Fragment>
-      <withRouter(AddTaskButton)
+      <withRouterIfEnabled(AddTaskButton)
         isDisabled={false}
         onTaskModalClose={[Function]}
         taskFormProps={

src/elements/content-sidebar/versions/VersionsSidebarContainer.js

@@ -37,7 +37,7 @@ type Props = {
     features: FeatureConfig,
     fileId: string,
     hasSidebarInitialized?: boolean,
-    history: RouterHistory,
+    history?: RouterHistory,
     internalSidebarNavigation?: InternalSidebarNavigation,
     internalSidebarNavigationHandler?: InternalSidebarNavigationHandler,
     match: Match,
@@ -284,7 +284,7 @@ class VersionsSidebarContainer extends React.Component<Props, State> {
                 delete navigationUpdate.versionId;
             }
             internalSidebarNavigationHandler(navigationUpdate);
-        } else {
+        } else if (history) {
             history.push(generatePath(match.path, { ...match.params, versionId }));
         }
     };