PanelStack: onOpen, onClose and initialPanel prop should use type argument P

[cecton]

Environment

• Package version(s): 3.30.0
• Browser and OS versions: Firefox 79, Arch Linux

Feature request

The arguments addedPanel and removedPanel of the functions onOpen and onClose on the interface IPanelStackProps don't allow setting their type argument. Therefore it is set to {}.

This is very inconvenient because then the type IPanel<{}> must be used for the state. Could you add a type parameter P to IPanelStackProps and use it in those methods? (You probably want to use it for the initialPanel too.)

Examples

The following code doesn't compile:

function Menubar(props: MenubarProps) {
  const [panels, setPanels] = useState<IPanel<MenubarProps>[]>([ // the type of the state is invalid
    {
      component: Settings,
      props,
      title: "Settings",
    },
  ]);

  return (
    <PanelStack
      className="Menubar"
      initialPanel={panels[0]}
      onOpen={(new_) => setPanels([new_, ...panels])}
      onClose={() => setPanels(panels.slice(1))}
    />
  );
}

This is the error:

TypeScript error in /home/cecile/repos/third-i-frontend/src/Menubar.tsx(39,35):
Argument of type '(IPanel<MenubarProps> | IPanel<{}>)[]' is not assignable to parameter of type 'SetStateAction<IPanel<MenubarProps>[]>'.
  Type '(IPanel<MenubarProps> | IPanel<{}>)[]' is not assignable to type 'IPanel<MenubarProps>[]'.
    Type 'IPanel<MenubarProps> | IPanel<{}>' is not assignable to type 'IPanel<MenubarProps>'.
      Type 'IPanel<{}>' is not assignable to type 'IPanel<MenubarProps>'.
        Types of property 'component' are incompatible.
          Type 'ComponentType<IPanelProps>' is not assignable to type 'ComponentType<PhotoMode & Network & IPanelProps>'.
            Type 'ComponentClass<IPanelProps, any>' is not assignable to type 'ComponentType<PhotoMode & Network & IPanelProps>'.
              Type 'ComponentClass<IPanelProps, any>' is not assignable to type 'ComponentClass<PhotoMode & Network & IPanelProps, any>'.
                Construct signature return types 'Component<IPanelProps, any, any>' and 'Component<PhotoMode & Network & IPanelProps, any, any>' are incompatible.
                  The types of 'props' are incompatible between these types.
                    Type 'Readonly<IPanelProps> & Readonly<{ children?: ReactNode; }>' is not assignable to type 'Readonly<PhotoMode & Network & IPanelProps> & Readonly<{ children?: ReactNode; }>'.
                      Type 'Readonly<IPanelProps> & Readonly<{ children?: ReactNode; }>' is missing the following properties from type 'Readonly<PhotoMode & Network & IPanelProps>': photoMode, setPhotoMode, setNetwork  TS2345

    37 |       className="Menubar"
    38 |       initialPanel={panels[0]}
  > 39 |       onOpen={(new_) => setPanels([new_, ...panels])}
       |                                   ^
    40 |       onClose={() => setPanels(panels.slice(1))}
    41 |     />
    42 |   );

As you can see: onOpen seems to be the culprit.

Here is a way to fix it (this code compiles):

function Menubar(props: MenubarProps) {
  const [panels, setPanels] = useState<(IPanel<MenubarProps>| IPanel)[]>([ // it works
    {
      component: Settings,
      props,
      title: "Settings",
    },
  ]);

  return (
    <PanelStack
      className="Menubar"
      initialPanel={panels[0]}
      onOpen={(new_) => setPanels([new_, ...panels])}
      onClose={() => setPanels(panels.slice(1))}
    />
  );
}

Here is another way:

Here I cast to IPanel<MenubarProps> in onOpen to circumvent the issue. The code compiles and the types are better:

function Menubar(props: MenubarProps) {
  const [panels, setPanels] = useState<IPanel<MenubarProps>[]>([
    {
      component: Settings,
      props,
      title: "Settings",
    },
  ]);

  return (
    <PanelStack
      className="Menubar"
      initialPanel={panels[0]}
      onOpen={(new_) => setPanels([new_ as IPanel<MenubarProps>, ...panels])} // cast to correct type
      onClose={() => setPanels(panels.slice(1))}
    />
  );
}

[adidahiya]

I agree with your assessment. The component should support panel type parameterization rather than defaulting to {}. I would be open to a PR which adds this feature. It would have to be added in a backwards compatible way, and we'd need some way for users to instantiate a PanelStack of a particular type, similar to Select.ofType<T>().