patternfly/patternfly-react-seed

Make the DynamicImport a TS generic to avoid swallowing the component's props

riccardo-forina opened this issue · 0 comments

Right now we suggest doing something like this

const SomePage = (routeProps: RouteComponentProps) => {
  const lastNavigation = useLastLocation();
  return (
    <DynamicImport load={getSomePageModuleAsync()} focusContentAfterMount={lastNavigation !== null}>
      {(Component: any) => {
        if (Component === null) {
          return (
            <PageSection aria-label="Loading Content Container">
              <div className="pf-l-bullseye">
                <Alert title="Loading" className="pf-l-bullseye__item" />
              </div>
            </PageSection>
          );
        } else {
          return <Component />; // we are not passing a prop that is required, but the TS compiler can't know this because we lost the component's types in the process!
        }
      }}
    </DynamicImport>
  );
};

Component is typed as any, which makes it impossible to make sure the right props are being passed to the component.
It would be nice specifying the interface the dynamically loaded component should respect, something like this:

interface SomePageRoute {
  foo: string;
}
const SomePage = (routeProps: RouteComponentProps<SomePageRoute>) => {
  const lastNavigation = useLastLocation();
  return (
    <DynamicImport<ISomePageProps> load={getSomePageAsync()} focusContentAfterMount={lastNavigation !== null}>
      {(Component) => {
        // Component here would be the actual component class/function, with the right type. 
        if (Component === null) {
          return (
            <PageSection aria-label="Loading Content Container">
              <div className="pf-l-bullseye">
                <Alert title="Loading" className="pf-l-bullseye__item" />
              </div>
            </PageSection>
          );
        } else {
          return <Component someRequiredPropFromTheRouter={routeProps.match.params.foo} />;
        }
      }}
    </DynamicImport>
  );
};

It doesn't sound like much, but this way the compiler can help you avoiding gluing a component to a route that doesn't provide the required data to instantiate it.