feat(ui): Remove lazy renderer component prop (#69823)

Author: scttcper

static/app/components/events/eventReplay/index.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useCallback} from 'react';
+import {Fragment, lazy} from 'react';
 import ReactLazyLoad from 'react-lazyload';
 import styled from '@emotion/styled';
 
@@ -36,6 +36,10 @@ export const REPLAY_CLIP_OFFSETS = {
   durationBeforeMs: 5_000,
 };
 
+const ReplayOnboardingPanel = lazy(() => import('./replayInlineOnboardingPanel'));
+const ReplayPreview = lazy(() => import('./replayPreview'));
+const ReplayClipPreview = lazy(() => import('./replayClipPreview'));
+
 function EventReplayContent({
   event,
   group,
@@ -46,13 +50,6 @@ function EventReplayContent({
   const router = useRouter();
   const {getReplayCountForIssue} = useReplayCountForIssues();
 
-  const replayOnboardingPanel = useCallback(
-    () => import('./replayInlineOnboardingPanel'),
-    []
-  );
-  const replayPreview = useCallback(() => import('./replayPreview'), []);
-  const replayClipPreview = useCallback(() => import('./replayClipPreview'), []);
-
   const hasReplayClipFeature = organization.features.includes(
     'issue-details-inline-replay-viewer'
   );
@@ -69,7 +66,7 @@ function EventReplayContent({
     return (
       <ErrorBoundary mini>
         <LazyLoad
-          component={replayOnboardingPanel}
+          LazyComponent={ReplayOnboardingPanel}
           platform={platform}
           projectId={projectId}
         />
@@ -148,12 +145,12 @@ function EventReplayContent({
             {hasReplayClipFeature ? (
               <LazyLoad
                 {...commonProps}
-                component={replayClipPreview}
+                LazyComponent={ReplayClipPreview}
                 clipOffsets={REPLAY_CLIP_OFFSETS}
                 overlayContent={overlayContent}
               />
             ) : (
-              <LazyLoad {...commonProps} component={replayPreview} />
+              <LazyLoad {...commonProps} LazyComponent={ReplayPreview} />
             )}
           </ReactLazyLoad>
         </ReplayGroupContextProvider>

static/app/components/lazyLoad.spec.tsx

@@ -1,3 +1,5 @@
+import {lazy} from 'react';
+
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import LazyLoad from 'sentry/components/lazyLoad';
@@ -15,7 +17,6 @@ function BarComponent({}: TestProps) {
 }
 
 type ResolvedComponent = {default: React.ComponentType<TestProps>};
-type GetComponent = () => Promise<ResolvedComponent>;
 
 describe('LazyLoad', function () {
   afterEach(() => {
@@ -25,7 +26,7 @@ describe('LazyLoad', function () {
   it('renders with a loading indicator when promise is not resolved yet', function () {
     const importTest = new Promise<ResolvedComponent>(() => {});
     const getComponent = () => importTest;
-    render(<LazyLoad component={getComponent} />);
+    render(<LazyLoad LazyComponent={lazy(getComponent)} />);
 
     // Should be loading
     expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
@@ -37,7 +38,7 @@ describe('LazyLoad', function () {
       doResolve = resolve;
     });
 
-    render(<LazyLoad component={() => importFoo} />);
+    render(<LazyLoad LazyComponent={lazy(() => importFoo)} />);
 
     // Should be loading
     expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
@@ -58,7 +59,7 @@ describe('LazyLoad', function () {
     );
 
     try {
-      render(<LazyLoad component={getComponent} />);
+      render(<LazyLoad LazyComponent={lazy(getComponent)} />);
     } catch (err) {
       // ignore
     }
@@ -78,7 +79,7 @@ describe('LazyLoad', function () {
     });
 
     // First render Foo
-    const {rerender} = render(<LazyLoad component={() => importFoo} />);
+    const {rerender} = render(<LazyLoad LazyComponent={lazy(() => importFoo)} />);
     expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
 
     // resolve with foo
@@ -90,22 +91,21 @@ describe('LazyLoad', function () {
       doResolve = resolve;
     });
 
-    rerender(<LazyLoad component={() => importBar} />);
+    rerender(<LazyLoad LazyComponent={lazy(() => importBar)} />);
     expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
 
     // resolve with bar
     doResolve!({default: BarComponent});
     expect(await screen.findByText('my bar component')).toBeInTheDocument();
 
     // Update component prop to a mock to make sure it isn't re-called
-    const getComponent2: GetComponent = jest.fn(
-      () => new Promise<ResolvedComponent>(() => {})
-    );
-    rerender(<LazyLoad component={getComponent2} />);
+    const getComponent2 = jest.fn(() => new Promise<ResolvedComponent>(() => {}));
+    const LazyGet = lazy(getComponent2);
+    rerender(<LazyLoad LazyComponent={LazyGet} />);
     expect(getComponent2).toHaveBeenCalledTimes(1);
 
     // Does not refetch on other prop changes
-    rerender(<LazyLoad component={getComponent2} testProp />);
+    rerender(<LazyLoad LazyComponent={LazyGet} testProp />);
     expect(getComponent2).toHaveBeenCalledTimes(1);
   });
 });

static/app/components/lazyLoad.tsx

@@ -1,29 +1,19 @@
 import type {ErrorInfo} from 'react';
-import {Component, lazy, Suspense, useMemo} from 'react';
+import {Component, Suspense} from 'react';
 import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
 
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {t} from 'sentry/locale';
 import {isWebpackChunkLoadingError} from 'sentry/utils';
-import retryableImport from 'sentry/utils/retryableImport';
-
-type PromisedImport<C> = Promise<{default: C}>;
-
-type ComponentType = React.ComponentType<any>;
-
-type Props<C extends ComponentType> = React.ComponentProps<C> & {
-  /**
-   * Wrap the component with lazy() before passing it to LazyLoad.
-   */
-  LazyComponent?: React.LazyExoticComponent<C>;
 
+type Props<C extends React.LazyExoticComponent<C>> = React.ComponentProps<C> & {
   /**
-   * Accepts a function to trigger the import resolution of the component.
-   * @deprecated Use `LazyComponent` instead and keep lazy() calls out of the render path.
+   * Wrap the component with `lazy()` before passing it to LazyLoad.
+   * This should be declared outside of the render funciton.
    */
-  component?: () => PromisedImport<C>;
+  LazyComponent: C;
 
   /**
    * Override the default fallback component.
@@ -42,20 +32,11 @@ type Props<C extends ComponentType> = React.ComponentProps<C> & {
  *
  * <LazyLoad LazyComponent={LazyComponent} someComponentProps={...} />
  */
-function LazyLoad<C extends ComponentType>({
-  component,
-  loadingFallback,
+function LazyLoad<C extends React.LazyExoticComponent<any>>({
   LazyComponent,
+  loadingFallback,
   ...props
 }: Props<C>) {
-  const LazyLoadedComponent = useMemo(() => {
-    if (LazyComponent) {
-      return LazyComponent;
-    }
-
-    return lazy<C>(() => retryableImport(component));
-  }, [component, LazyComponent]);
-
   return (
     <ErrorBoundary>
       <Suspense
@@ -67,7 +48,8 @@ function LazyLoad<C extends ComponentType>({
           )
         }
       >
-        <LazyLoadedComponent {...(props as React.ComponentProps<C>)} />
+        {/* Props are strongly typed when passed in, but seem to conflict with LazyExoticComponent */}
+        <LazyComponent {...(props as any)} />
       </Suspense>
     </ErrorBoundary>
   );

static/app/routes.tsx

@@ -28,7 +28,10 @@ import {IndexRoute, Route} from './components/route';
 
 const hook = (name: HookName) => HookStore.get(name).map(cb => cb());
 
-const SafeLazyLoad = errorHandler(LazyLoad);
+// LazyExoticComponent Props get crazy when wrapped in an additional layer
+const SafeLazyLoad = errorHandler(LazyLoad) as unknown as React.ComponentType<
+  typeof LazyLoad
+>;
 
 // NOTE: makeLazyloadComponent is exported for use in the sentry.io (getsentry)
 // pirvate routing tree.