shell: More types than ever #21426

mvollmer · 2024-12-12T13:34:32Z

shell: Even more types #21425

A manifest is now just a JsonObject, and users are expected to type-cast it (or parts) of it to more concrete types, like the new ShellManifest and ManifestParentSection.

The cockpit.spawn function takes only two arguments (unlike cockpit.script). Location.reload() doesn't take any arguments, the "force_reload" argument is a Firefox extension.

A CockpitNav component wants the "filtering" function to put a keyword on each item, but the the item can be anything otherwise. CockpitHosts uses it with Machine objects, and PageNav with ManifestItems.

It doesn't happen anymore, yay!

mvollmer · 2024-12-12T13:35:28Z

This is still another approach, just rename everything (almost) and get it relaxed green.

pkg/shell/nav.tsx

+    current: string;
+}
+
+export class CockpitNav<T> extends React.Component {


cockpituous · 2024-12-12T15:11:30Z

pkg/lib/cockpit-components-table.tsx

+            if (col && typeof col == "object") {
+                const props = col as ListingTableRowColumnProps;
+                if (props.sortKey)
+                    return props.sortKey;
+                if (typeof props.title == "string")
+                    return props.title;


These 6 added lines are not executed by any test.

cockpituous · 2024-12-12T15:11:30Z

pkg/lib/cockpit-components-table.tsx

+                if (typeof props.title == "string")
+                    return props.title;
+            }
+            return "";


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:30Z

pkg/lib/cockpit-components-table.tsx

-                                    {typeof cell == 'object' ? cell.title : cell}
+                                <Th key={key || `row_${rowKey}_cell_${colKey}`} dataLabel={dataLabel}
+                                    {...cellProps as ThProps}>
+                                    {typeof cell == 'object' ? cell_as_props.title : cell}


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:31Z

pkg/shell/credentials.tsx

@@ -215,7 +116,7 @@ const PublicKey = ({ currentKey }) => {

 const KeyPassword = ({ currentKey, change, setDialogError }) => {
    const [confirmPassword, setConfirmPassword] = useState('');
-    const [inProgress, setInProgress] = useState(undefined);
+    const [inProgress, setInProgress] = useState<boolean | undefined>(undefined);


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:31Z

pkg/shell/credentials.tsx

@@ -245,8 +146,7 @@
    const changePasswordBtn = (
        <Button variant="primary"
                id={(currentKey.name || currentKey.comment) + "-change-password"}
-                isDisabled={inProgress}
-                isLoading={inProgress}
+                {... inProgress !== undefined ? { isDisabled: inProgress, isLoading: inProgress } : {} }


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:33Z

pkg/shell/shell-modals.tsx

-                                            .filter(key => !searchInput || manifest.locales[key].toLowerCase().includes(searchInput.toString().toLowerCase()));
+                                    const locales = manifest.locales || {};
+                                    const filteredLocales = Object.keys(locales)
+                                            .filter(key => !searchInput || locales[key].toLowerCase().includes(searchInput.toString().toLowerCase()));


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:33Z

pkg/shell/shell.tsx

+            arg.then(function(...args) { console.log(...args) },
+                     function(...args) { console.error(...args) });


These 2 added lines are not executed by any test.

cockpituous · 2024-12-12T15:11:33Z

pkg/shell/shell.tsx

            if (typeof arg.stream == "function")
-                arg.stream(function() { console.log.apply(console, arguments) });
+                arg.stream(function(...args) { console.log(...args) });


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:34Z

pkg/shell/topnav.tsx

                {cockpit.format(_("$0 documentation"), this.state.osRelease.NAME)}
            </DropdownItem>);

+        const shell_manifest = (cockpit.manifests.shell || {}) as ShellManifest;


This added line is not executed by any test.

cockpituous · 2024-12-12T15:11:34Z

pkg/shell/topnav.tsx

        // global documentation for cockpit as a whole
-        (cockpit.manifests.shell?.docs ?? []).forEach(doc => {
+        (shell_manifest.docs ?? []).forEach(doc => {


This added line is not executed by any test.

martinpitt · 2024-12-18T15:28:48Z

@mvollmer unblocked!

mvollmer added 12 commits December 12, 2024 13:03

lib: Type ListingTable

02a14ce

shell: Improve manifest types

1086679

A manifest is now just a JsonObject, and users are expected to type-cast it (or parts) of it to more concrete types, like the new ShellManifest and ManifestParentSection.

shell: Fully type "nav"

22b9475

shell: Fully type "topnav"

6a6e06d

shell: Check types in "shell-modals"

3fe8a88

shell: Fix two bugs found by TypeScript

a628346

The cockpit.spawn function takes only two arguments (unlike cockpit.script). Location.reload() doesn't take any arguments, the "force_reload" argument is a Firefox extension.

shell: Mass rename files to "tsx"

a0e119d

shell: Make CockpitNav type generic

7f0afaa

A CockpitNav component wants the "filtering" function to put a keyword on each item, but the the item can be anything otherwise. CockpitHosts uses it with Machine objects, and PageNav with ManifestItems.

lib: Add types for cockpit.sessionStorage and cockpit.localStorage

3bdd7b6

WIP - roughshot typing

4a0f478

WIP - fix bugs found by typescript

2a393cf

test: Don't ignore TS2740

d415bcc

It doesn't happen anymore, yay!

mvollmer added the blocked Don't land until something else happens first (see task list) label Dec 12, 2024

github-advanced-security bot found potential problems Dec 12, 2024

View reviewed changes

pkg/shell/nav.tsx

current: string;

}

export class CockpitNav<T> extends React.Component {

Check warning

Code scanning / CodeQL

Unused or undefined state property Warning

Component state property 'current' is

written

Loading
, but it is never read.

cockpituous reviewed Dec 12, 2024

View reviewed changes

martinpitt removed the blocked Don't land until something else happens first (see task list) label Dec 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shell: More types than ever #21426

shell: More types than ever #21426

mvollmer commented Dec 12, 2024 •

edited by martinpitt

Loading

mvollmer commented Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

cockpituous Dec 12, 2024

martinpitt commented Dec 18, 2024

		arg.then(function(...args) { console.log(...args) },
		function(...args) { console.error(...args) });

shell: More types than ever #21426

Are you sure you want to change the base?

shell: More types than ever #21426

Conversation

mvollmer commented Dec 12, 2024 • edited by martinpitt Loading

mvollmer commented Dec 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Dec 18, 2024

mvollmer commented Dec 12, 2024 •

edited by martinpitt

Loading