ts-for-gir icon indicating copy to clipboard operation
ts-for-gir copied to clipboard

When constructing `St.Widget`, specifying `layout_manager` (as a subclass of `LayoutManager`) should be reflected in types

Open KSXGitHub opened this issue 6 months ago • 2 comments

From https://github.com/gjsify/gnome-shell/issues/18


When one constructs a St.Widget like this:

const myWidget = new St.Widget({
  layout_manager: new Clutter.GridLayout(),
})

The type of myWidget.layout_manager is still LayoutManager, which would make TypeScript errors when one tries to use methods from GridLayout.

Suggestion 1: Multiple type parameters

// St
class Widget<LayoutManagerT extends Clutter.LayoutManager | null = null, LabelActorT extends Clutter.LabelActor | null = null> extends Clutter.Actor<LayoutManagerT, LabelActorT> {
  constructor(config?: Widget.ConstructorProperties<LayoutManagerT, LabelActorT>)
}

// Clutter
interface Actor<LayoutManagerT extends LayoutManager | null, LabelActorT extends LabelActor | null> {
  layout_manager: LayoutManagerT
  layoutManager: LayoutManagerT
  label_actor: LabelActorT
  labelActor: LabelActorT
}

[!WARNING] ConstructorProperties now shouldn't be a single interface, but a union of snake_case properties and camelCase properties. Keeping it as a single interface would have required user to specify both layout_manager and layoutManager, which would be absurd.

Suggestion 2: Type dictionary as a single type parameter

// St
class Widget<Config extends Widget.ConstructorProperties | null> extends ClutterActor<Config> {
  constructor(config: Config)
}

// Clutter
interface Actor<Config extends Widget.ConstructorProperties | null> {
  layout_manager: Config extends Widget.ConstructorProperties ? Config['layout_manager'] : null
  layoutManager: Config extends Widget.ConstructorProperties ? Config['layoutManager'] : null
  label_actor: Config extends Widget.ConstructorProperties ? Config['label_actor'] : null
  labelActor: Config extends Widget.ConstructorProperties ? Config['labelActor'] : null
}

[!NOTE] I strongly recommend this solution. ConstructorProperties does not need to change at all.


The two suggestions above still have a flaw: It now requires TypeScript user to pass null explicitly, optional parameter is no longer allowed.

If you want to fix this flaw, then I suggest stop using the class keyword. Instead, you should utilize the TypeScript feature that allows merging variable and type, like so (assuming you choose "suggestion 2"):

const Widget: unknown
  & (new <Config extends Widget.ConstructorProperties> (config: Config) => Widget<Config>)
  & (new (config?: null) => Widget<null>)
interface Widget<Config extends Widget.ConstructorProperties | null> extends Widget.Actor<Config> {}

As for me personally, whether I have to pass null explicitly makes little difference.

KSXGitHub avatar Feb 11 '24 12:02 KSXGitHub