nomcopter / react-mosaic

A React tiling window manager
https://nomcopter.github.io/react-mosaic/
Other
4.39k stars 226 forks source link

Add children to the MosaicWindowProps #186

Closed JR000 closed 2 years ago

JR000 commented 2 years ago

Fixes #184

Changes proposed in this pull request:

In React versions lower than 18 there was no need for the сhildren property to be in the MosaicWindowProps, because the props itself was defined in @types/react/index.d.ts as:

readonly props: Readonly<P> & Readonly<{ children?: ReactNode | undefined }>;

But in React 18 there is no default support for children:

readonly props: Readonly<P>;

Since a MosaicWindow usually has children, children?: ReactNode | undefined should be defined in the MosaicWindowProps explicilty. Otherwise using the package with React 18 and trying to add children to a MosaicWindow we get the following error:

No overload matches this call.
  Overload 1 of 2, '(props: MosaicWindowProps<string> | Readonly<MosaicWindowProps<string>>): MosaicWindow<string>', gave the following error.
    Type '{ children: Element; path: MosaicBranch[]; title: string; toolbarControls: undefined[]; draggable: true; createNode: () => string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MosaicWindow<string>> & Readonly<MosaicWindowProps<string>>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MosaicWindow<string>> & Readonly<MosaicWindowProps<string>>'.
  Overload 2 of 2, '(props: MosaicWindowProps<string>, context: any): MosaicWindow<string>', gave the following error.
    Type '{ children: Element; path: MosaicBranch[]; title: string; toolbarControls: undefined[]; draggable: true; createNode: () => string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MosaicWindow<string>> & Readonly<MosaicWindowProps<string>>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MosaicWindow<string>> & Readonly<MosaicWindowProps<string>>'
Wozniaxos commented 2 years ago

@nomcopter Can we merge it? it looks like a non-breaking fix.

nomcopter commented 2 years ago

I plan to get this in soon! Just need to find time to batch some of this work together and cut a release - thanks for your patience.

nomcopter commented 2 years ago

Folded into v5.2.0 - thanks!