Aylur / ags

A customizable and extensible shell
GNU General Public License v3.0
2.19k stars 114 forks source link

Wrong transformFn type in Binding #262

Closed MariaSolOs closed 9 months ago

MariaSolOs commented 9 months ago

The parameter type of this function looks incorrect.

To see why, here's a snippet of my configuration:

start_widget: Widget.Box({
  class_name: Battery.bind('percent').transform(p => {
    if (p < 20) return 'pink-box';
    if (p < 40) return 'yellow-box';
    return 'green-box';
  }),
  ...

This generates the following type error:

Type 'Binding<Battery, "percent", "pink-box" | "yellow-box" | "green-box">' is not assignable to type 'string | Binding<any, any, string> | undefined'.
  Type 'Binding<Battery, "percent", "pink-box" | "yellow-box" | "green-box">' is not assignable to type 'Binding<any, any, string>'.
    Types of property 'transformFn' are incompatible.
      Type '(v: "pink-box" | "yellow-box" | "green-box") => "pink-box" | "yellow-box" | "green-box"' is not assignable to type '(v: string) => string'.
        Types of parameters 'v' and 'v' are incompatible.
          Type 'string' is not assignable to type '"pink-box" | "yellow-box" | "green-box"'. [2322]
Aylur commented 9 months ago

This is an issue I have been struggling to solve for quite some time

const bind = new Binding<any, any, 'hello'>(battery, 'percent');
const asd = 'hello' as const;

Label({
     // ts knows that 'hello' type can be assigned to a string
    label: asd,
});

Label({
    // it doesn't know that Binding<any, any, 'hello'> can be
    // assigned to Binding<any, any, string>
    label: bind,
});

I assume this requires some deeper understanding of typescript that I lack

I usually work around it by forcing it to infer it as a string instead of a tuple

class_name: Battery.bind('percent').transform(p => {
    let r = 'green-box';
    if (p < 20)
        r = 'pink-box';
    if (p < 40)
        r = 'yellow-box';
    return r;
}),

but if its a widget for example there is no workaround

MariaSolOs commented 9 months ago

@Aylur that's fair. I work in TypeScript and I'm not sure how to fix it myself either lol.

MariaSolOs commented 8 months ago

@Aylur I'm still having the error appear with the latest Arch release. I'm not sure if this is because I need to do something to update the types though.

EDIT: Nvm, I just had to run ags --init and remove the old types and node_modules folders.

J4ckTh3R1pper commented 8 months ago

I'm still having this issue on latest git build fa59075, console outputs below:

(com.github.Aylur.ags:85274): Gjs-CRITICAL **: 14:59:29.302: JS ERROR: TypeError: b is undefined
batteryProgress<.tooltip_text<@file:///home/dayp308/.config/ags/bar.js:99:48
transform/bind.transformFn@resource:///com/github/Aylur/ags/service.js:14:37
bind/callback<@resource:///com/github/Aylur/ags/widgets/widget.js:60:30
hook/<@resource:///com/github/Aylur/ags/widgets/widget.js:50:25
_init/GLib.MainLoop.prototype.runAsync/</<@resource:///org/gnome/gjs/modules/core/overrides/GLib.js:266:34

and my widget code:

const batteryProgress = Widget.Box({
    "visible": battery.bind('available'),
    "spacing": 0,
    "tooltip_text": battery.bind().transform(b => b["percent"]), // this is the line where error occurs
    "children": [
        Widget.ProgressBar({
            "show-text": false,
            "vpack": "center"
        }).hook(battery, self => {
            self["css"] = `
                trough progress {
                    background-color: ${
                        battery.charging ? "#00C000" :
                        battery.percent <= 14 ? "FF0000" :
                        "#FFFFFF"
                    };
                    margin: 0px;
                    min-height: 10px;
                    min-width: 17px;
                }
                trough {
                    min-height: 10px;
                    min-width: 17px;
                }
            `
            self["value"] = battery.percent > 0 ? battery.percent / 100 : 0,
            self["class-name"] = battery.charging ? 'charging' : ''
        }),
        ]
})
Aylur commented 8 months ago

@J4ckTh3R1pper your issue is unrelated to this one. you just forgot to specify the property in the bind() call