Skip to content

Remove ts-strict-ignore and fix TypeScript errors [structures] #6108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lkostrowski wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Remove ts-strict-ignore and fix TypeScript errors [structures] #6108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a8e20e
fix(structures): remove @ts-strict-ignore and fix TypeScript strict m…
claude Nov 22, 2025
565fffd
fix: add runtime null checks and normalization for GraphQL menu items
claude Nov 22, 2025
395a52f
fix: use explicit typing for normalizeMenuItems function
claude Nov 22, 2025
a350c2e
fix: add null/undefined checks for MenuList and MenuDetails
claude Nov 22, 2025
5de7ee2
Merge branch 'main' into claude/fix-typescript-strict-01PN9fmdHRPEuXJ…
lkostrowski Nov 25, 2025
56d8b9b
fixes
lkostrowski Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/structures/components/MenuDetailsPage/MenuDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-strict-ignore
import { TopNav } from "@dashboard/components/AppLayout";
import { ConfirmButtonTransitionState } from "@dashboard/components/ConfirmButton";
import Form from "@dashboard/components/Form";
Expand All @@ -11,12 +10,13 @@ import { MenuDetailsFragment, MenuErrorFragment } from "@dashboard/graphql";
import { SubmitPromise } from "@dashboard/hooks/useForm";
import useNavigator from "@dashboard/hooks/useNavigator";
import { menuListUrl } from "@dashboard/structures/urls";
import { UniqueIdentifier } from "@dnd-kit/core";
import { useState } from "react";

import { MenuItemType } from "../MenuItemDialog";
import MenuItems, { TreeOperation } from "../MenuItems";
import MenuProperties from "../MenuProperties";
import { computeRelativeTree } from "./tree";
import { computeRelativeTree, normalizeMenuItems } from "./tree";

export interface MenuDetailsFormData {
name: string;
Expand All @@ -33,10 +33,10 @@ interface MenuDetailsPageProps {
menu: MenuDetailsFragment;
onDelete: () => void;
onItemAdd: () => void;
onItemClick: (id: string, type: MenuItemType) => void;
onItemEdit: (id: string) => void;
onItemClick: (id: UniqueIdentifier, type: MenuItemType) => void;
onItemEdit: (id: UniqueIdentifier) => void;
// If not passed, it will not render the button. Use to control permissions
onTranslate?: (id: string) => void;
onTranslate?: (id: UniqueIdentifier) => void;
onSubmit: (data: MenuDetailsSubmitData) => SubmitPromise;
}

Expand Down Expand Up @@ -96,7 +96,11 @@ const MenuDetailsPage = ({
<DetailPageLayout.Content>
<MenuItems
canUndo={treeOperations.length > 0}
items={menu?.items ? computeRelativeTree(menu.items, treeOperations) : []}
items={
menu?.items
? computeRelativeTree(normalizeMenuItems(menu.items), treeOperations)
: []
}
onChange={handleChange}
onItemAdd={onItemAdd}
onItemClick={onItemClick}
Expand Down
60 changes: 45 additions & 15 deletions src/structures/components/MenuDetailsPage/tree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// @ts-strict-ignore
import { RecursiveMenuItem } from "@dashboard/structures/types";

import { menu } from "../../fixtures";
import { TreeOperation } from "../MenuItems";
import { computeRelativeTree } from "./tree";
import { computeRelativeTree, normalizeMenuItems } from "./tree";

// Normalize menu items for testing
const normalizedMenuItems = normalizeMenuItems(menu.items);

const relativeOutput: RecursiveMenuItem[][] = [
// no moves
Expand Down Expand Up @@ -1156,7 +1158,7 @@ function innerTreeToString(tree: RecursiveMenuItem, level: number): string {
"\n" +
"··".repeat(level) +
tree.name +
tree.children.reduce((acc, node) => acc + innerTreeToString(node, level + 1), "")
tree.children?.reduce((acc, node) => acc + innerTreeToString(node, level + 1), "")
);
}

Expand All @@ -1167,66 +1169,94 @@ function treeToString(tree: RecursiveMenuItem[]): string {
describe("Properly computes trees", () => {
testTable.forEach(testData =>
it("#", () => {
const computedTree = computeRelativeTree(menu.items, testData);
const computedTree = computeRelativeTree(normalizedMenuItems, testData);

expect(treeToString(computedTree)).toMatchSnapshot();
}),
);
});
describe("Properly computes relative trees", () => {
it("doesn't move anything", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[0]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[0]);

expect(computedTree).toEqual(relativeOutput[0]);
});
it("moves one root element", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[1]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[1]);

expect(computedTree).toEqual(relativeOutput[1]);
});
it("moves two root element", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[2]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[2]);

expect(computedTree).toEqual(relativeOutput[2]);
});
it("empty moves", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[3]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[3]);

expect(computedTree).toEqual(relativeOutput[3]);
});
it("moves every element", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[4]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[4]);

expect(computedTree).toEqual(relativeOutput[4]);
});
it("moves children", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[5]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[5]);

expect(computedTree).toEqual(relativeOutput[5]);
});
it("moves child outside", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[6]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[6]);

expect(computedTree).toEqual(relativeOutput[6]);
});
it("moves child outside and puts it in a location", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[7]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[7]);

expect(computedTree).toEqual(relativeOutput[7]);
});
it("moves child inside", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[8]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[8]);

expect(computedTree).toEqual(relativeOutput[8]);
});
it("moves child inside then outside then changes index", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[9]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[9]);

expect(computedTree).toEqual(relativeOutput[9]);
});
it("moves item as last child and moves it up", () => {
const computedTree = computeRelativeTree(menu.items, secondTestTable[10]);
const computedTree = computeRelativeTree(normalizedMenuItems, secondTestTable[10]);

expect(computedTree).toEqual(relativeOutput[10]);
});
});
describe("Error handling for invalid paths", () => {
it("throws error when trying to move non-existent node", () => {
// Arrange
const invalidOperation: TreeOperation = {
id: "non-existent-id",
type: "move",
sortOrder: 0,
};

// Act & Assert
expect(() => {
computeRelativeTree(normalizedMenuItems, [invalidOperation]);
}).toThrow("Cannot get node with null path");
});

it("throws error when trying to remove non-existent node", () => {
// Arrange
const invalidOperation: TreeOperation = {
id: "non-existent-id",
type: "remove",
};

// Act & Assert
expect(() => {
computeRelativeTree(normalizedMenuItems, [invalidOperation]);
}).toThrow("Cannot get node with null path");
});
});
63 changes: 48 additions & 15 deletions src/structures/components/MenuDetailsPage/tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
// @ts-strict-ignore
import { RecursiveMenuItem } from "@dashboard/structures/types";

import { TreeOperation } from "../MenuItems";

export function findNode(tree: RecursiveMenuItem[], id: string): number[] {
// Normalizes GraphQL menu items to ensure children is always an array

export function normalizeMenuItems(items: any): RecursiveMenuItem[] {
if (!items) return [];

return items.map(
(item: any): RecursiveMenuItem => ({
...item,
children: normalizeMenuItems(item.children),
}),
);
}
Comment on lines +7 to +16
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normalizeMenuItems function uses any type for its input parameter, which defeats the purpose of TypeScript strict mode. Consider using a more specific type that represents the GraphQL response structure or a generic type that extends the expected shape.

Suggested improvement:

export function normalizeMenuItems(items: unknown): RecursiveMenuItem[] {
  if (!items || !Array.isArray(items)) return [];

  return items.map((item: any): RecursiveMenuItem => ({
    ...item,
    children: normalizeMenuItems(item.children),
  }));
}

Or better yet, define a proper input type based on the GraphQL schema.

Copilot uses AI. Check for mistakes.

export function findNode(tree: RecursiveMenuItem[], id: string): (number | null)[] {
const foundNodeIndex = tree.findIndex(node => node.id === id);

if (tree.length === 0) {
Expand All @@ -14,39 +26,49 @@ export function findNode(tree: RecursiveMenuItem[], id: string): number[] {
return [foundNodeIndex];
}

const nodeMap = tree.map((node, nodeIndex) => [nodeIndex, ...findNode(node.children, id)]);
const nodeMap = tree.map((node, nodeIndex) => [nodeIndex, ...findNode(node.children ?? [], id)]);

return nodeMap.find(path => path[path.length - 1] !== null) || [null];
}

export function getNode(tree: RecursiveMenuItem[], path: number[]): RecursiveMenuItem {
export function getNode(tree: RecursiveMenuItem[], path: (number | null)[]): RecursiveMenuItem {
const index = path[0];

if (index === null) {
throw new Error("Cannot get node with null path");
}

if (path.length === 1) {
return tree[path[0]];
return tree[index];
}

return getNode([...tree[path[0]].children], path.slice(1));
return getNode(tree[index].children ?? [], path.slice(1));
}

function removeNode(tree: RecursiveMenuItem[], path: number[]): RecursiveMenuItem[] {
function removeNode(tree: RecursiveMenuItem[], path: (number | null)[]): RecursiveMenuItem[] {
const removeIndex = path[0];

if (removeIndex === null) {
throw new Error("Cannot remove node with null path");
}

if (path.length === 1) {
return [...tree.slice(0, removeIndex), ...tree.slice(removeIndex + 1)];
}

const newTree = [...tree];

newTree[removeIndex] = {
...tree[path[0]],
children: removeNode(tree[path[0]].children, path.slice(1)),
...tree[removeIndex],
children: removeNode(tree[removeIndex].children ?? [], path.slice(1)),
};

return newTree;
}

interface InsertNodeInput {
tree: RecursiveMenuItem[];
path: number[];
path: (number | null)[];
node: RecursiveMenuItem;
position: number;
}
Expand All @@ -56,9 +78,15 @@ function insertNode({ tree, path, node, position }: InsertNodeInput): RecursiveM
return [...tree.slice(0, position), node, ...tree.slice(position)];
}

if (path[0] in tree) {
tree[path[0]].children = insertNode({
tree: tree[path[0]].children,
const index = path[0];

if (index === null) {
throw new Error("Cannot insert node with null path");
}

if (index in tree) {
tree[index].children = insertNode({
tree: tree[index].children ?? [],
path: path.slice(1),
node,
position,
Expand Down Expand Up @@ -99,13 +127,18 @@ function permuteRelativeNode(
const node = getNode(tree, sourcePath);
const hasParent = !!permutation.parentId;
const treeAfterRemoval = removeNode(tree, sourcePath);
const targetPath = hasParent ? findNode(treeAfterRemoval, permutation.parentId) : [];
const targetPath = hasParent ? findNode(treeAfterRemoval, permutation.parentId ?? "") : [];
const position = sourcePath[sourcePath.length - 1];

if (position === null) {
throw new Error("Cannot permute node with null position");
}

const treeAfterInsertion = insertNode({
tree: treeAfterRemoval,
path: targetPath,
node,
position: position + permutation.sortOrder,
position: position + (permutation.sortOrder ?? 0),
});

return treeAfterInsertion;
Expand Down
1 change: 0 additions & 1 deletion src/structures/components/MenuItemDialog/MenuItemDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-strict-ignore
import BackButton from "@dashboard/components/BackButton";
import { Combobox } from "@dashboard/components/Combobox";
import { ConfirmButton, ConfirmButtonTransitionState } from "@dashboard/components/ConfirmButton";
Expand Down
12 changes: 6 additions & 6 deletions src/structures/components/MenuItemDialog/MenuItemDialogLinkValue/useLinkValue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ export const useLinkValue = (linkType: MenuItemTypeWithOptions) => {
const pages = mapEdgesToItems(pageSearch?.result?.data?.search) || [];

const categoriesOptions = categories?.map(category => ({
value: category.id,
label: category.name,
value: (category as { id: string }).id,
label: (category as { name: string }).name,
}));
const collectionsOptions = collections?.map(collection => ({
value: collection.id,
label: collection.name,
value: (collection as { id: string }).id,
label: (collection as { name: string }).name,
}));
const pagesOptions = pages?.map(page => ({
value: page.id,
label: page.title,
value: (page as { id: string }).id,
label: (page as { title: string }).title,
}));
Comment on lines 48 to 59
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The type assertions for accessing properties on mapped items are repetitive and overly specific. Since mapEdgesToItems is a generic function that preserves the type T, these assertions suggest that the search results are not properly typed.

Consider either:

  1. Properly typing the search results at their source (in the search hooks) so that TypeScript knows what properties are available
  2. If type assertions are necessary, extract them into a helper function to reduce repetition

For example:

type EntityWithId = { id: string };
type EntityWithName = { name: string };
type PageEntity = { id: string; title: string };

const categoriesOptions = categories?.map(category => ({
  value: (category as EntityWithId).id,
  label: (category as EntityWithName).name,
}));

Copilot uses AI. Check for mistakes.

const options = {
Expand Down
8 changes: 4 additions & 4 deletions src/structures/components/MenuItems/MenuItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-strict-ignore
import { DashboardCard } from "@dashboard/components/Card";
import { buttonMessages } from "@dashboard/intl";
import { RecursiveMenuItem } from "@dashboard/structures/types";
import { UniqueIdentifier } from "@dnd-kit/core";
import { Box, Button, Skeleton } from "@saleor/macaw-ui-next";
import { useMemo } from "react";
import { FormattedMessage, useIntl } from "react-intl";
Expand All @@ -16,10 +16,10 @@ interface MenuItemsProps {
items: RecursiveMenuItem[];
onChange: (operations: TreeOperation[]) => void;
onItemAdd: () => void;
onItemClick: (id: string, type: MenuItemType) => void;
onItemEdit: (id: string) => void;
onItemClick: (id: UniqueIdentifier, type: MenuItemType) => void;
onItemEdit: (id: UniqueIdentifier) => void;
// If not passed, it will not render the button. Use to control permissions
onTranslate?: (id: string) => void;
onTranslate?: (id: UniqueIdentifier) => void;
onUndo: () => void;
}

Expand Down
1 change: 0 additions & 1 deletion src/structures/components/MenuItems/tree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-strict-ignore
import { MenuItemFragment } from "@dashboard/graphql";
import { MenuTreeItem } from "@dashboard/structures/types";

Expand Down
2 changes: 0 additions & 2 deletions src/structures/components/MenuItems/tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-strict-ignore

import { MenuTreeItem } from "@dashboard/structures/types";
import { getPatch } from "fast-array-diff";

Expand Down
Loading
Loading