Gr3q / types-cjs

Typescript declarations for CJS - Cinnamon JavaScript
MIT License
5 stars 2 forks source link

St-1.0.d.ts, class ThemeNode, wrong type in method #21

Open kriegcc opened 2 months ago

kriegcc commented 2 months ago

Hi,

there is a small type issue I stumbled upon: imports.gi.St.ThemeNode (St-1.0.d.ts)

class ThemeNode {
public constructor(options?: Partial<ThemeNodeInitOptions>);
/**
    * Creates a new {@link ThemeNode}. Once created, a node is immutable. Of any
    * of the attributes of the node (like the #element_class) change the node
    * and its child nodes must be destroyed and recreated.
    * @param context the context representing global state for this themed tree
    * @param parent_node the parent node of this node
    * @param theme a theme (stylesheet set) that overrides the
    *   theme inherited from the parent node
    * @param element_type the type of the GObject represented by this node
    *  in the tree (corresponding to an element if we were theming an XML
    *  document. %G_TYPE_NONE means this style was created for the stage
    * actor and matches a selector element name of 'stage'.
    * @param element_id the ID to match CSS rules against
    * @param element_class a whitespace-separated list of classes
    *   to match CSS rules against
    * @param pseudo_class a whitespace-separated list of pseudo-classes
    *   (like 'hover' or 'visited') to match CSS rules against
    * @param inline_style
    * @param important
    * @returns the theme node
    */
public static new(context: ThemeContext, parent_node: ThemeNode | null, theme: Theme | null, element_type: GObject.Type, element_id: string | null, element_class: string | null, pseudo_class: string | null, inline_style: string, important: boolean): ThemeNode;
}

In static method new, the parameter element_type has a wrong type. It is at the moment GObject.Type (which is a number) but actually should be of type GObject.GType, I think:

    public static new(
      context: ThemeContext,
      parent_node: ThemeNode | null,
      theme: Theme | null,
      element_type: GObject.GType,
      element_id: string | null,
      element_class: string | null,
      pseudo_class: string | null,
      inline_style: string,
      important: boolean,
    ): ThemeNode

As far as I understand, this file is generated by "GIR2TS". So it does not make sense to provide a fix here? The "gir" file used to generate this type has somehow defined the correct type: https://github.com/Gr3q/GIR2TS/blob/001449996f43adb84394cf784d5d7abd9f343e44/gir/St-1.0.gir#L7802

Thanks!

Gr3q commented 2 months ago

I see you forked my spaghetti.

It's converted here https://github.com/Gr3q/GIR2TS/blob/master/src/utils/paramUtils.ts#L45 because as far as I can see GObject.GType doesn't exist in the generated GObject-2.0.d.ts.

Still, https://github.com/gjsify/ts-for-gir/blob/main/packages/lib/src/transformation.ts#L152 ts-for-gir converts it to GType but I'll be honest I've not used GTypes enough nor I read ts-for-gir's code that deeply to know the best way to do this.

If you could provide usage examples I might be figure out a generic rule on how to fix this.

kriegcc commented 2 months ago

Thanks for your reply!

Here is an example how I use the ThemeNode.new method:

Some util function: https://github.com/kriegcc/cinnamon-spices-applets/blob/ca1c5277ec84633245d1fab7df87570b25d7a878/fish%40kriegcc/src/utils/common/theme.ts#L29

export function getThemeNodeOfClass(className: string): imports.gi.St.ThemeNode {
  const themeContext = imports.gi.St.ThemeContext.get_for_stage(global.stage)
  const themeNode = imports.gi.St.ThemeNode.new(
    themeContext,
    null,
    themeContext.get_theme(),
    imports.gi.St.Widget,
    "",
    className,
    "",
    // "inline_style": undocumented argument.
    // Bug: Using empty string here causes an error in console. See: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4634
    " ",
    // "important": undocumented argument.
    // Fallback lookup in default Cinnamon theme, I guess. Seems to be something CJS specific (argument is not in GJS).
    true,
  )
  return themeNode
}

Usage within an applet class: https://github.com/kriegcc/cinnamon-spices-applets/blob/ca1c5277ec84633245d1fab7df87570b25d7a878/fish%40kriegcc/src/FishApplet/FishApplet.ts#L637

  private getAppletMargin(): number {
    const themeNode = getThemeNodeOfClass(DEFAULT_APPLET_CLASS_NAME)
    const margin =
      themeNode.get_horizontal_padding() +
      themeNode.get_border_width(imports.gi.St.Side.TOP) +
      themeNode.get_border_width(imports.gi.St.Side.BOTTOM)
    return margin
  }

This basically allows to read styles for an element (here "applet-box" class) before they are on stage.

That is actually the only place where I use GTypes.

kriegcc commented 2 months ago

A cheap fix would be to do a manual override, I think.

Actually, I needed some additional methods on ThemeNode for which there were no declarations yet (but there were on IThemeNode so I just copied them: https://github.com/kriegcc/cinnamon-spices-applets/blob/ca1c5277ec84633245d1fab7df87570b25d7a878/fish%40kriegcc/src/types/ci-types-additions/St.d.ts

Gr3q commented 2 months ago

Ok, now I see.

I think GObject.Type is just any class (not instantiated) that inherits from GObject.Object. In an ideal world if I specify the type as typeof GObject.Object instead of GObject.Type it would represent what it's intended for.

For example if you create your ThemeNode, then use get_element_type, what you should get is ThemeNode in that case. So if you cast that to ThemeNode (for now because currently that is also GObject.Type you should be able to create another ThemeNode instance from it at runtime.

The problem is because I had to use mixins to represent multi-class inheritance which don't exist in Typescript they only appearing to be inheriting from one another, when you actually use them as types they fail the constraint checks.

I'll try a few things, maybe I can improve the mixins.

Gr3q commented 2 months ago
    export type ObjectType<T extends ObjectMixin = ObjectMixin> = {
        new (): T
    };

Something like this might work for GType, but it might fall apart if you substitute it to something else, like Clutter.ActorMixin

kriegcc commented 2 months ago

Thanks for your investigation and explanation! I have to admit that I am not that deep into GObject introspection yet.

At the moment, I use the manual overwrite which seems to work fine. But of course it would be nice to get rid of it someday if that's possible.