Skip to content

Commit

Permalink
fix(web): make TypeDoc works again (#1832)
Browse files Browse the repository at this point in the history
# Problem

After updating to TypeDoc 0.27, the `npm run typedoc` task stop working
because problems when loading `typedoc-plugin-external-module-map`. See
https://github.com/agama-project/agama/actions/runs/12302232612/job/34334622800

## Solution

Open an [issue in
upstream](asgerjensen/typedoc-plugin-external-module-map#27)
and stop using the conflicting plugin, fix all TypeDoc issues (see next
section), and revert commit d946480
added at #1225

## Additional work

Even without the mentioned plugin, TypeDoc tasks still failing because
other crash already mentioned in the commit message of
791857a

> TypeError: Cannot read properties of undefined (reading 'kindOf')
> at
/home/runner/work/agama/agama/web/node_modules/typedoc/dist/lib/models/types.js:687:63

Migrating some components to TypeScript solved the issue. This was done
at commit
34bc000,
which also includes adjustments needed after these migrations.
  • Loading branch information
dgdavid authored Dec 16, 2024
2 parents 6284cbf + a84849c commit 4aee8fb
Show file tree
Hide file tree
Showing 22 changed files with 215 additions and 534 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/github-pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
run: cd web && npm ci

- name: Build Web UI documentation
run: cd web && npm run typedoc:client && mv typedoc.out/ ../doc/dist/web-ui
run: cd web && npm run typedoc && mv typedoc.out/ ../doc/dist/web-ui

- name: Setup Pages
uses: actions/configure-pages@v3
Expand Down
31 changes: 0 additions & 31 deletions web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@
"ts-loader": "^9.5.1",
"tsconfig-paths-webpack-plugin": "^4.0.0",
"typedoc": "^0.27.4",
"typedoc-plugin-external-module-map": "^2.1.0",
"typedoc-plugin-merge-modules": "^6.0.0",
"typedoc-plugin-missing-exports": "^3.0.0",
"typescript": "^5.7.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,13 @@ import React from "react";
import { screen, within } from "@testing-library/react";
import { plainRender } from "~/test-utils";
import { ExpandableSelector } from "~/components/core";
import { ExpandableSelectorColumn } from "./ExpandableSelector";

const sda = {
let consoleErrorSpy: jest.SpyInstance;

/* eslint-disable @typescript-eslint/no-explicit-any */

const sda: any = {
sid: "59",
isDrive: true,
type: "disk",
Expand Down Expand Up @@ -112,18 +117,20 @@ const vg = {
lvs: [lv1],
};

const columns = [
{ name: "Device", value: (item) => item.name },
const columns: ExpandableSelectorColumn[] = [
// FIXME: do not use any but the right types once storage part is rewritten.
// Or even better, write a test not coupled to storage
{ name: "Device", value: (item: any) => item.name },
{
name: "Content",
value: (item) => {
value: (item: any) => {
if (item.isDrive) return item.systems.map((s, i) => <p key={i}>{s}</p>);
if (item.type === "vg") return `${item.lvs.length} logical volume(s)`;

return item.content;
},
},
{ name: "Size", value: (item) => item.size },
{ name: "Size", value: (item: any) => item.size },
];

const onChangeFn = jest.fn();
Expand All @@ -141,11 +148,12 @@ const commonProps = {

describe("ExpandableSelector", () => {
beforeAll(() => {
jest.spyOn(console, "error").mockImplementation();
consoleErrorSpy = jest.spyOn(console, "error");
consoleErrorSpy.mockImplementation();
});

afterAll(() => {
console.error.mockRestore();
consoleErrorSpy.mockRestore();
});

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@
* find current contact information at www.suse.com.
*/

// @ts-check

import React, { useState } from "react";
import {
Table,
TableProps,
Thead,
Tr,
Th,
Expand All @@ -34,10 +33,7 @@ import {
RowSelectVariant,
} from "@patternfly/react-table";

/**
* @typedef {import("@patternfly/react-table").TableProps} TableProps
* @typedef {import("react").RefAttributes<HTMLTableElement>} HTMLTableProps
*/
/* eslint-disable @typescript-eslint/no-explicit-any */

/**
* An object for sharing data across nested maps
Expand All @@ -47,26 +43,48 @@ import {
* places, as it is the case of the rowIndex prop here.
*
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions#passing_arguments
*
* @typedef {object} SharedData
* @property {number} rowIndex - The current row index, to be incremented each time a table row is generated.
*/

/**
* @typedef {object} ExpandableSelectorColumn
* @property {string} name - The column header text.
* @property {(object) => React.ReactNode} value - A function receiving
* the item to work with and returning the column value.
* @property {string} [classNames] - space-separated list of additional CSS class names.
*/
type SharedData = {
rowIndex: number;
};

export type ExpandableSelectorColumn = {
/** The column header text */
name: string;
/** A function receiving the item to work with and returns the column value */
value: (item: object) => React.ReactNode;
/** Space-separated list of additional CSS class names */
classNames?: string;
};

export type ExpandableSelectorProps = {
/** Collection of objects defining columns. */
columns?: ExpandableSelectorColumn[];
/** Whether multiple selection is allowed. */
isMultiple?: boolean;
/** Collection of items to be rendered. */
items?: object[];
/** The key for retrieving the item id. */
itemIdKey?: string;
/** Lookup method to retrieve children from given item. */
itemChildren?: (item: object) => object[];
/** Whether an item will be selectable or not. */
itemSelectable?: (item: object) => boolean;
/** Callback to add additional CSS class names to item row. */
itemClassNames?: (item: object) => string | undefined;
/** Collection of selected items. */
itemsSelected?: object[];
/** Ids of initially expanded items. */
initialExpandedKeys?: any[];
/** Callback to be triggered when selection changes. */
onSelectionChange?: (selection: object[]) => void;
} & TableProps;

/**
* Internal component for building the table header
*
* @param {object} props
* @param {ExpandableSelectorColumn[]} props.columns
*/
const TableHeader = ({ columns }) => (
const TableHeader = ({ columns }: { columns: ExpandableSelectorColumn[] }) => (
<Thead noWrap>
<Tr>
<Th />
Expand All @@ -86,14 +104,14 @@ const TableHeader = ({ columns }) => (
* It logs information to console.error if given value does not match
* expectations.
*
* @param {*} selection - The value to check.
* @param {boolean} allowMultiple - Whether the returned collection can have
* @param selection - The value to check.
* @param allowMultiple - Whether the returned collection can have
* more than one item
* @return {Array} Empty array if given value is not valid. The first element if
* @return Empty array if given value is not valid. The first element if
* it is a collection with more than one but selector does not allow multiple.
* The original value otherwise.
*/
const sanitizeSelection = (selection, allowMultiple) => {
const sanitizeSelection = (selection: any[], allowMultiple: boolean): any[] => {
if (!Array.isArray(selection)) {
console.error("`itemSelected` prop must be an array. Ignoring given value", selection);
return [];
Expand All @@ -117,20 +135,6 @@ const sanitizeSelection = (selection, allowMultiple) => {
*
* @note It only accepts one nesting level.
*
* @typedef {object} ExpandableSelectorBaseProps
* @property {ExpandableSelectorColumn[]} [columns=[]] - Collection of objects defining columns.
* @property {boolean} [isMultiple=false] - Whether multiple selection is allowed.
* @property {object[]} [items=[]] - Collection of items to be rendered.
* @property {string} [itemIdKey="id"] - The key for retrieving the item id.
* @property {(item: object) => Array<object>} [itemChildren=() => []] - Lookup method to retrieve children from given item.
* @property {(item: object) => boolean} [itemSelectable=() => true] - Whether an item will be selectable or not.
* @property {(item: object) => (string|undefined)} [itemClassNames=() => ""] - Callback that allows adding additional CSS class names to item row.
* @property {object[]} [itemsSelected=[]] - Collection of selected items.
* @property {any[]} [initialExpandedKeys=[]] - Ids of initially expanded items.
* @property {(selection: Array<object>) => void} [onSelectionChange=noop] - Callback to be triggered when selection changes.
*
* @typedef {ExpandableSelectorBaseProps & TableProps & HTMLTableProps} ExpandableSelectorProps
*
* @param {ExpandableSelectorProps} props
*/
export default function ExpandableSelector({
Expand All @@ -145,10 +149,10 @@ export default function ExpandableSelector({
initialExpandedKeys = [],
onSelectionChange,
...tableProps
}) {
}: ExpandableSelectorProps) {
const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys);
const selection = sanitizeSelection(itemsSelected, isMultiple);
const isItemSelected = (item) => {
const isItemSelected = (item: object) => {
const selected = selection.find((selectionItem) => {
return (
Object.hasOwn(selectionItem, itemIdKey) && selectionItem[itemIdKey] === item[itemIdKey]
Expand All @@ -157,16 +161,16 @@ export default function ExpandableSelector({

return selected !== undefined || selection.includes(item);
};
const isItemExpanded = (key) => expandedItemsKeys.includes(key);
const toggleExpanded = (key) => {
const isItemExpanded = (key: string | number) => expandedItemsKeys.includes(key);
const toggleExpanded = (key: string | number) => {
if (isItemExpanded(key)) {
setExpandedItemsKeys(expandedItemsKeys.filter((k) => k !== key));
} else {
setExpandedItemsKeys([...expandedItemsKeys, key]);
}
};

const updateSelection = (item) => {
const updateSelection = (item: object) => {
if (!isMultiple) {
onSelectionChange([item]);
return;
Expand All @@ -182,11 +186,11 @@ export default function ExpandableSelector({
/**
* Render method for building the markup for an item child
*
* @param {object} item - The child to be rendered
* @param {boolean} isExpanded - Whether the child should be shown or not
* @param {SharedData} sharedData - An object holding shared data
* @param item - The child to be rendered
* @param isExpanded - Whether the child should be shown or not
* @param sharedData - An object holding shared data
*/
const renderItemChild = (item, isExpanded, sharedData) => {
const renderItemChild = (item: object, isExpanded: boolean, sharedData: SharedData) => {
const rowIndex = sharedData.rowIndex++;

const selectProps = {
Expand All @@ -212,10 +216,10 @@ export default function ExpandableSelector({
/**
* Render method for building the markup for item
*
* @param {object} item - The item to be rendered
* @param {SharedData} sharedData - An object holding shared data
* @param item - The item to be rendered
* @param sharedData - An object holding shared data
*/
const renderItem = (item, sharedData) => {
const renderItem = (item: object, sharedData: SharedData) => {
const itemKey = item[itemIdKey];
const rowIndex = sharedData.rowIndex++;
const children = itemChildren(item);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2023] SUSE LLC
* Copyright (c) [2023-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand All @@ -24,7 +24,7 @@ import React, { useState } from "react";
import { screen } from "@testing-library/react";
import { plainRender } from "~/test-utils";
import userEvent from "@testing-library/user-event";
import PasswordInput from "./PasswordInput";
import PasswordInput, { PasswordInputProps } from "./PasswordInput";
import { _ } from "~/i18n";

describe("PasswordInput Component", () => {
Expand Down Expand Up @@ -58,7 +58,7 @@ describe("PasswordInput Component", () => {
// Using a controlled component for testing the rendered result instead of testing if
// the given onChange callback is called. The former is more aligned with the
// React Testing Library principles, https://testing-library.com/docs/guiding-principles/
const PasswordInputTest = (props) => {
const PasswordInputTest = (props: PasswordInputProps) => {
const [password, setPassword] = useState(null);

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2023] SUSE LLC
* Copyright (c) [2023-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand All @@ -20,29 +20,26 @@
* find current contact information at www.suse.com.
*/

// @ts-check

import React, { useState } from "react";
import { Button, InputGroup, TextInput } from "@patternfly/react-core";
import { Button, InputGroup, TextInput, TextInputProps } from "@patternfly/react-core";
import { _ } from "~/i18n";
import { Icon } from "~/components/layout";

/**
* @typedef {import("@patternfly/react-core").TextInputProps} TextInputProps
*
* Props matching the {@link https://www.patternfly.org/components/forms/text-input PF/TextInput},
* except `type` that will be forced to 'password'.
* @typedef {Omit<TextInputProps, 'type'> & { inputRef?: React.Ref<HTMLInputElement> }} PasswordInputProps
*/
export type PasswordInputProps = Omit<TextInputProps, "type"> & {
inputRef?: React.Ref<HTMLInputElement>;
};

/**
* Renders a password input field and a toggle button that can be used to reveal
* and hide the password
* @component
*
* @param {PasswordInputProps} props
*/
export default function PasswordInput({ id, inputRef, ...props }) {
export default function PasswordInput({ id, inputRef, ...props }: PasswordInputProps) {
const [showPassword, setShowPassword] = useState(false);
const visibilityIconName = showPassword ? "visibility_off" : "visibility";

Expand Down
Loading

0 comments on commit 4aee8fb

Please sign in to comment.