Proptypes cleanups by jelly · Pull Request #23223 · cockpit-project/cockpit

jelly · 2026-05-06T18:00:38Z

No description provided.

The default <Flex> creates the expected layout.

mvollmer · 2026-05-11T06:31:50Z

            return a.localeCompare(b);
        });
-        const restarts = daemons.map(service => cockpit.spawn(["systemctl", "restart", service], { superuser: "required", err: "message" }));
+        const restarts = daemons.map(service => cockpit.spawn(["systemctl", "restart", service], { superuser: "require", err: "message" }));


Ouch! But anything truthy other than "try" is effectively treated as "require", right?

Yes, I believe so. But I can't find the relevant code in the bridge

mvollmer · 2026-05-11T06:57:14Z

+    activeTab,
+    tabErrors
+}: {
+    onChange: (tab: string) => void;


Tab: TabKey?

mvollmer · 2026-05-11T07:19:20Z

    return (
        <Nav variant="horizontal-subnav" id="services-filter"
-             onSelect={(_event, result) => { setActiveItem(result.itemId); onChange(result.itemId) }}>
+             onSelect={(_event, result) => { setActiveItem(result.itemId as TabKey); onChange(String(result.itemId)) }}>


As usual, I am unhappy about the "as" here, but I think short of making Nav generic we have no choice.

mvollmer · 2026-05-11T07:29:29Z

+             onSelect={(_event, result) => { setActiveItem(result.itemId as TabKey); onChange(String(result.itemId)) }}>
            <NavList>
-                {Object.keys(service_tabs).map(key => {
+                {(Object.keys(service_tabs) as TabKey[]).map(key => {


Unhappy about this "as" as well, but: (I think we can shorting this introduction to "asbut" :-)

I tried to improve this, but it is somewhat involved. The best I could come up with is using a Map instead of a Record. It was a valuable learning experience, so let me share:

If we want proper type inference, we need to use a Map and Map.prototype.keys. Constructing the map is a bit inelegant, and we should move it to the global scope to avoid constructing it all the time:

const service_tabs: Map<TabKey, string> = new Map([ [ "service", _("Services") ], [ "socket", _("Sockets") ], [ "target", _("Targets") ], [ "timer", _("Timers") ], [ "path", _("Paths") ], ]);

So now we should be able to write service_tabs.keys().map(k => ...) and k should be of type TabKey.

Except that Iterator,prototype.map is only in ES2025 which we probably don't want to depend on just for this. (We would also need to bump our TypeScript version.)

So we could write [...service_tabs.keys()].map(k => ...), but that creates an array at run time. To be fair Object.keys() also creates an array, no?

So we could have another global just to list the keys:

const service_tabs_keys = [...service_tabs.keys()];

and then use service_tabs_keys.map(k => ...).

That would be kinda ok, but is it worth it? I don't know.

We currently target es2021 in our TypeScript, I had to do some digging and finally found prototype.map which is baseline 2025. So fairly recent and we should target Firefox ESR as minimum which is Firefox 140.

mvollmer · 2026-05-11T07:50:27Z

+interface TabRenderer {
+    name: string;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    renderer: React.ComponentType<any>;


React.ComponentType<unknown> works as well and doesn't require silencing the error.

Not, 100%

pkg/shell/credentials.tsx(314,51): error TS2322: Type '({ data: { currentKey: Key; keys?: never; setDialogError?: never; }; name: string; renderer: ({ currentKey }: { currentKey: Key; }) => Element; } | { data: { currentKey: Key; keys: Keys; setDialogError: Dispatch<...>; }; name: string; renderer: ({ currentKey, keys, setDialogError }: { ...; }) => Element; })[]' is not assignable to type 'TabRenderer[]'. Type '{ data: { currentKey: Key; keys?: never; setDialogError?: never; }; name: string; renderer: ({ currentKey }: { currentKey: Key; }) => Element; } | { data: { currentKey: Key; keys: Keys; setDialogError: Dispatch<...>; }; name: string; renderer: ({ currentKey, keys, setDialogError }: { ...; }) => Element; }' is not assignable to type 'TabRenderer'. Type '{ data: { currentKey: credentials.Key; keys?: never; setDialogError?: never; }; name: string; renderer: ({ currentKey }: { currentKey: credentials.Key; }) => React.JSX.Element; }' is not assignable to type 'TabRenderer'. Types of property 'renderer' are incompatible. Type '({ currentKey }: { currentKey: credentials.Key; }) => React.JSX.Element' is not assignable to type 'ComponentType<unknown>'. Type '({ currentKey }: { currentKey: credentials.Key; }) => React.JSX.Element' is not assignable to type 'FunctionComponent<unknown>'. Types of parameters '__0' and 'props' are incompatible. Type 'unknown' is not assignable to type '{ currentKey: Key; }'.

mvollmer · 2026-05-11T07:51:05Z

+}
+
+interface ListingPanelState {
+    activeTab: number;


Let's just make this string | number then we can omit the tabIndex = Number(tabIndex) below.

mvollmer

Thanks! I think we should do the changes that I have proposed for ListingPanel.

Assisted-By: Claude Opus 4.6

Assisted-By: Claude Opus

For this to be converted to TypeScript first services.jsx has to be converted which is a non-trivial amount of work. Adding jsdoc is also not trivial due to to the need to define `unit`.

mvollmer

Thanks!

jelly · 2026-05-13T13:38:14Z

RHEL 8 packagekit tests are impossible to get green.

jelly added 2 commits May 6, 2026 11:20

treewide: Fix superuser mode required to require

329a8f8

systemd: remove unknown flex value property

23420d4

The default <Flex> creates the expected layout.

jelly requested review from mvollmer and tomasmatus May 7, 2026 08:16

mvollmer reviewed May 11, 2026

View reviewed changes

mvollmer requested changes May 11, 2026

View reviewed changes

systemd: Port service-tabs to TypeScript

2da77b7

Assisted-By: Claude Opus 4.6

jelly force-pushed the proptypes-cleanups branch from f2c0f4e to beec208 Compare May 11, 2026 18:34

github-advanced-security AI found potential problems May 11, 2026

View reviewed changes

Comment thread pkg/lib/cockpit-components-listing-panel.tsx Fixed

jelly added 2 commits May 11, 2026 20:47

lib: Port cockpit-components-listing-panel to TypeScript

799a0c4

Assisted-By: Claude Opus

systemd: drop propTypes for ServiceDetails

74f70b5

For this to be converted to TypeScript first services.jsx has to be converted which is a non-trivial amount of work. Adding jsdoc is also not trivial due to to the need to define `unit`.

jelly force-pushed the proptypes-cleanups branch from beec208 to 74f70b5 Compare May 11, 2026 18:47

jelly requested a review from mvollmer May 12, 2026 14:14

mvollmer approved these changes May 13, 2026

View reviewed changes

jelly merged commit 2ff6080 into cockpit-project:main May 13, 2026
89 of 91 checks passed

jelly deleted the proptypes-cleanups branch May 13, 2026 17:27

Conversation

jelly commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvollmer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mvollmer left a comment

Choose a reason for hiding this comment

Uh oh!

jelly commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants