feat(router): withRouterIfEnabled (#4221)

* feat(router): withRouterIfEnabled

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

* feat(router): withRouterIfEnabled - fixes

* feat(router): withRouterIfEnabled - fixes

* feat(router): withRouterIfEnabled - fixes

Author: rafalmaksymiuk

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

@@ -0,0 +1,55 @@
+// @flow
+import * as React from 'react';
+import { MemoryRouter } from 'react-router-dom';
+import { render } from '../../../../test-utils/testing-library';
+import withRouterIfEnabled from '../withRouterIfEnabled';
+
+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 component = getByTestId('test-component');
+    expect(component).toHaveAttribute('data-has-history', 'true');
+    expect(component).toHaveAttribute('data-has-location', 'true');
+    expect(component).toHaveAttribute('data-has-match', 'true');
+    expect(component).not.toHaveAttribute('data-router-disabled');
+});
+
+test('renders without Router and without router props (routerDisabled prop)', () => {
+    const { getByTestId } = render(<WithRouterIfEnabled routerDisabled />);
+    const component = getByTestId('test-component');
+    expect(component).not.toHaveAttribute('data-has-history');
+    expect(component).not.toHaveAttribute('data-has-location');
+    expect(component).not.toHaveAttribute('data-has-match');
+    expect(component).toHaveAttribute('data-router-disabled', 'true');
+});
+
+test('renders without Router and without router props (feature flag)', () => {
+    const features = { routerDisabled: { value: true } };
+    const { getByTestId } = render(<WithRouterIfEnabled features={features} />);
+
+    const component = getByTestId('test-component');
+    expect(component).not.toHaveAttribute('data-has-history');
+    expect(component).not.toHaveAttribute('data-has-location');
+    expect(component).not.toHaveAttribute('data-has-match');
+});
+

src/elements/common/routing/index.js

@@ -1,2 +1,3 @@
-// eslint-disable-next-line import/prefer-default-export
+// @flow
 export { default as withRouterAndRef } from './withRouterAndRef';
+export { default as withRouterIfEnabled } from './withRouterIfEnabled';

src/elements/common/routing/withRouterIfEnabled.js

@@ -0,0 +1,24 @@
+// @flow
+import * as React from 'react';
+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;
+
+        return <Component {...props} />;
+    };
+
+    const name = Wrapped.displayName || Wrapped.name || 'Component';
+    WithRouterIfEnabled.displayName = `withRouterIfEnabled(${name})`;
+
+    return WithRouterIfEnabled;
+}
+

src/elements/content-sidebar/AddTaskButton.js

@@ -1,6 +1,7 @@
 // @flow
 import * as React from 'react';
-import { withRouter, type RouterHistory } from 'react-router-dom';
+import { type RouterHistory } from 'react-router-dom';
+import { withRouterIfEnabled } from '../common/routing';
 
 import AddTaskMenu from './AddTaskMenu';
 import TaskModal from './TaskModal';
@@ -11,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,
@@ -54,7 +55,7 @@ class AddTaskButton extends React.Component<Props, State> {
                 },
                 true,
             );
-        } else {
+        } else if (history) {
             history.replace({ state: { open: true } });
         }
 
@@ -103,4 +104,4 @@ class AddTaskButton extends React.Component<Props, State> {
 }
 
 export { AddTaskButton as AddTaskButtonComponent };
-export default withRouter(AddTaskButton);
+export default withRouterIfEnabled(AddTaskButton);

src/elements/content-sidebar/SidebarToggle.js

@@ -5,7 +5,8 @@
  */
 
 import * as React from 'react';
-import { withRouter, type RouterHistory } from 'react-router-dom';
+import { type RouterHistory } from 'react-router-dom';
+import { withRouterIfEnabled } from '../common/routing';
 import SidebarToggleButton from '../../components/sidebar-toggle-button/SidebarToggleButton';
 import { SIDEBAR_NAV_TARGETS } from '../common/interactionTargets';
 import type { InternalSidebarNavigation, InternalSidebarNavigationHandler } from '../common/types/SidebarNavigation';
@@ -53,4 +54,4 @@ const SidebarToggle = ({
 };
 
 export { SidebarToggle as SidebarToggleComponent };
-export default withRouter(SidebarToggle);
+export default withRouterIfEnabled(SidebarToggle);

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

@@ -9,7 +9,7 @@ import flow from 'lodash/flow';
 import getProp from 'lodash/get';
 import merge from 'lodash/merge';
 import noop from 'lodash/noop';
-import { generatePath, withRouter } from 'react-router-dom';
+import { generatePath } from 'react-router-dom';
 import type { Match, RouterHistory } from 'react-router-dom';
 import type { MessageDescriptor } from 'react-intl';
 import { withFeatureConsumer, isFeatureEnabled } from '../../common/feature-checking';
@@ -21,6 +21,7 @@ import StaticVersionsSidebar from './StaticVersionSidebar';
 import VersionsSidebar from './VersionsSidebar';
 import VersionsSidebarAPI from './VersionsSidebarAPI';
 import { withAPIContext } from '../../common/api-context';
+import { withRouterIfEnabled } from '../../common/routing';
 import type { FeatureConfig } from '../../common/feature-checking';
 import type { VersionActionCallback, VersionChangeCallback, SidebarLoadCallback } from './flowTypes';
 import type { BoxItemVersion, BoxItem, FileVersions } from '../../../common/types/core';
@@ -36,7 +37,7 @@ type Props = {
     features: FeatureConfig,
     fileId: string,
     hasSidebarInitialized?: boolean,
-    history: RouterHistory,
+    history?: RouterHistory,
     internalSidebarNavigation?: InternalSidebarNavigation,
     internalSidebarNavigationHandler?: InternalSidebarNavigationHandler,
     match: Match,
@@ -283,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 }));
         }
     };
@@ -349,4 +350,4 @@ class VersionsSidebarContainer extends React.Component<Props, State> {
 
 export type VersionsSidebarProps = Props;
 export { VersionsSidebarContainer as VersionsSidebarContainerComponent };
-export default flow([withRouter, withAPIContext, withFeatureConsumer])(VersionsSidebarContainer);
+export default flow([withRouterIfEnabled, withAPIContext, withFeatureConsumer])(VersionsSidebarContainer);