Skip to content
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.

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

chore: Fix no-unnecessary-condition linting errors #667

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion js/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export default tseslint.config(
// ------------------------------------------------------------------------------------
"react/no-array-index-key": "warn", // 6 errors
"@typescript-eslint/no-explicit-any": "warn", // 124 errors
"@typescript-eslint/no-unnecessary-condition": "warn", // 54 errors
"@typescript-eslint/no-unsafe-member-access": "warn", // 120 errors
"@typescript-eslint/no-misused-promises": "warn", // 26 errors
"@typescript-eslint/prefer-nullish-coalescing": "warn", // 133 errors
Expand Down
2 changes: 1 addition & 1 deletion js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"engines": {
"node": ">=18"
},
"packageManager": "[email protected].2+sha512.47870716bea1572b53df34ad8647b42962bc790ce2bf4562ba0f643237d7302a3d6a8ecef9e4bdfc01d23af1969aa90485d4cebb0b9638fa5ef1daef656f6c1b",
"packageManager": "[email protected].5+sha512.cdf928fca20832cd59ec53826492b7dc25dc524d4370b6b4adbf65803d32efaa6c1c88147c0ae4e8d579a6c9eec715757b50d4fa35eea179d868eada4ed043af",
"scripts": {
"dev": "next dev",
"build": "npm run clean && next build && mv out ../recce/data",
Expand Down
3 changes: 2 additions & 1 deletion js/src/components/app/EnvInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
} from "@chakra-ui/react";
import { format, parseISO } from "date-fns";
import { IconInfo } from "../icons";
import { isEmpty } from "lodash";

export function formatTimestamp(timestamp: string): string {
const date = parseISO(timestamp);
Expand Down Expand Up @@ -112,7 +113,7 @@ export function EnvInfo() {
</Link>
</ListItem>
)}
{reviewInfo && renderInfoEntries(reviewInfo)}
{!isEmpty(reviewInfo) && renderInfoEntries(reviewInfo)}
</UnorderedList>
</Flex>
</>
Expand Down
1 change: 1 addition & 0 deletions js/src/components/app/Filename.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
}

export const Filename = () => {
const { fileName, cloudMode, isDemoSite, isLoading } = useLineageGraphContext();

Check warning on line 96 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

'isLoading' is assigned a value but never used
const modalDisclosure = useDisclosure();
const overwriteDisclosure = useDisclosure();
const isStateless = !fileName && !cloudMode && !isDemoSite;
Expand All @@ -104,7 +104,7 @@

const [{ newFileName, errorMessage, modified, overwriteWithMethod, bypass }, setState] =
useState<FilenameState>({
newFileName: fileName || "recce_state.json",

Check warning on line 107 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator
});

const inputRef = useRef<HTMLInputElement>(null);
Expand All @@ -113,7 +113,7 @@

const handleOpen = () => {
setState({
newFileName: fileName || "recce_state.json",

Check warning on line 116 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator
modified: fileName ? false : true,
});

Expand All @@ -131,16 +131,16 @@
if (method === "save") {
await saveAs({
filename: newFileName,
overwrite: overwrite || bypassOverwrite,

Check warning on line 134 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator
});
} else {
await rename({
filename: newFileName,
overwrite: overwrite || bypassOverwrite,

Check warning on line 139 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator
});
}
toastSuccess(method === "save" ? "Save file successfully" : "Rename file successfully");
queryClient.invalidateQueries({ queryKey: cacheKeys.lineage() });

Check warning on line 143 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
if (bypass) {
localStorage.setItem(localStorageKeys.bypassSaveOverwrite, "true");
}
Expand Down Expand Up @@ -182,7 +182,8 @@
return (
<>
<Flex flex="1" justifyContent="center" alignItems="center">
{/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */}
<Box fontWeight="600">{fileName ? fileName : cloudMode ? "cloud" : titleNewInstance}</Box>

Check warning on line 186 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Prefer using nullish coalescing operator (`??`) instead of a ternary expression, as it is simpler to read
<Tooltip label={fileName ? "Change Filename" : "Save"} openDelay={1000}>
<IconButton onClick={handleOpen} aria-label={""} variant="unstyled" size="sm">
<Icon as={fileName ? IconEdit : IconSave} boxSize={"16px"} verticalAlign="middle" />
Expand Down Expand Up @@ -235,9 +236,9 @@
}

if (!fileName) {
handleAction("save");

Check warning on line 239 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
} else {
handleAction("rename");

Check warning on line 241 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}
} else if (e.key === "Escape") {
modalDisclosure.onClose();
Expand All @@ -252,7 +253,7 @@
size="sm"
colorScheme={fileName ? undefined : "blue"}
onClick={() => {
handleAction("save");

Check warning on line 256 in js/src/components/app/Filename.tsx

View workflow job for this annotation

GitHub Actions / eslint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}}
isDisabled={!newFileName || !!errorMessage || !modified}>
{fileName ? "Save as New File" : "Confirm"}
Expand Down
3 changes: 2 additions & 1 deletion js/src/components/charts/TopKSummaryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function prepareSummaryList(topK: TopKResult, isDisplayTopTen: boolean): Summary
if (isLastItemOthers) {
label = "(others)";
isSpecialLabel = true;
} else if (v === undefined || v === null) {
} else if (v == null) {
label = "(null)";
isSpecialLabel = true;
} else if (typeof v === "string" && v.length === 0) {
Expand Down Expand Up @@ -212,6 +212,7 @@ export function TopKSummaryList({ topk, valids, isDisplayTopTen }: Props) {
const displayTopkRatio = formatIntervalMinMax(topkCount / valids);
return (
<Fragment key={index}>
{/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */}
{!isLastItemOthers || (isLastItemOthers && topkCount > 0) ? (
<>
<Flex alignItems={"center"} width={"100%"} _hover={{ bg: "blackAlpha.300" }} px={3}>
Expand Down
12 changes: 6 additions & 6 deletions js/src/components/check/CheckBreadcrumb.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useRef, useState } from "react";
import React, { ChangeEvent, useCallback, useEffect, useRef, useState } from "react";
import { Box, Breadcrumb, BreadcrumbItem, Input } from "@chakra-ui/react";
import { ChevronRightIcon } from "@chakra-ui/icons";

Expand All @@ -10,7 +10,7 @@ interface CheckBreadcrumbProps {
export function CheckBreadcrumb({ name, setName }: CheckBreadcrumbProps) {
const [isEditing, setIsEditing] = useState(false);
const [editValue, setEditValue] = useState(name);
const editInputRef = useRef(null);
const editInputRef = useRef<HTMLInputElement>(null);

const handleClick = () => {
setEditValue(name);
Expand All @@ -32,13 +32,13 @@ export function CheckBreadcrumb({ name, setName }: CheckBreadcrumbProps) {
}
};

const handleChange: React.ChangeEventHandler = (event) => {
setEditValue((event.target as any).value);
const handleChange: React.ChangeEventHandler = (event: ChangeEvent<HTMLInputElement>) => {
setEditValue(event.target.value);
};

useEffect(() => {
const handleClickOutside = (event: any) => {
if (editInputRef.current && !(editInputRef.current as any).contains(event.target)) {
const handleClickOutside = (event: MouseEvent) => {
if (editInputRef.current && !editInputRef.current.contains(event.target as Node | null)) {
handleCommit();
}
};
Expand Down
3 changes: 2 additions & 1 deletion js/src/components/check/CheckDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ export const CheckDetail = ({ checkId, refreshCheckList }: CheckDetailProps) =>
}

const markdown = buildMarkdown(check);
if (!navigator.clipboard) {
// @see https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts
if (!window.isSecureContext) {
failToast(
"Failed to copy the check to clipboard",
new Error("Copy to clipboard is available only in secure contexts (HTTPS)"),
Expand Down
1 change: 1 addition & 0 deletions js/src/components/check/CheckList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const ChecklistItem = ({
};

const icon: IconType = findByRunType(check.type)?.icon || TbChecklist;
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
const isMarkAsApprovedDisabled = isDisabledByNoResult(check.type ?? "", run);

return (
Expand Down
3 changes: 2 additions & 1 deletion js/src/components/lineage/GraphNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,11 @@ export function GraphNode({ data }: GraphNodeProps) {
<Box height={`${data.columnSet.size * 15}px`} overflow="auto"></Box>
</Box>
)}

{/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */}
{Object.keys(data.parents ?? {}).length > 0 && (
<Handle type="target" position={Position.Left} isConnectable={false} />
)}
{/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */}
{Object.keys(data.children ?? {}).length > 0 && (
<Handle type="source" position={Position.Right} isConnectable={false} />
)}
Expand Down
7 changes: 3 additions & 4 deletions js/src/components/lineage/LineageViewTopBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,11 @@ const NodeSelectionInput = (props: {
}) => {
const [inputValue, setInputValue] = useState(props.value);
const { data: flags } = useRecceServerFlag();
const inputRef = useRef(null);
const inputRef = useRef<HTMLInputElement>(null);

useEffect(() => {
if (inputRef.current) {
(inputRef.current as any).value = props.value;
inputRef.current.value = props.value;
}
}, [props.value]);

Expand Down Expand Up @@ -282,7 +282,7 @@ const NodeSelectionInput = (props: {
event.preventDefault();
setInputValue(props.value);
if (inputRef.current) {
(inputRef.current as any).blur();
inputRef.current.blur();
}
}
}}
Expand Down Expand Up @@ -391,7 +391,6 @@ export const LineageViewTopBar = () => {
variant={"outline"}
size="xs"
fontSize="9pt"
isDisabled={selectMode !== "multi"}
onClick={() => {
deselect();
}}>
Expand Down
1 change: 1 addition & 0 deletions js/src/components/lineage/PresetCheckRecommendation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const usePresetCheckRecommendation = () => {
}
}

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (querySelectedModels.status === "success" && querySelectedModels.data) {
return querySelectedModels.data.nodes;
}
Expand Down
4 changes: 2 additions & 2 deletions js/src/components/lineage/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ export function getNeighborSet(
degree = 1000,
) {
const neighborSet = new Set<string>();
const visited: Record<string, number> = {};
const visited: Record<string, number | undefined> = {};

const dfs = (id: string, currentDegree: number) => {
if (currentDegree < 0) {
return;
}
if (visited[id] !== undefined && visited[id] >= currentDegree) {
if (visited[id] != null && visited[id] >= currentDegree) {
return;
}
visited[id] = currentDegree;
Expand Down
4 changes: 3 additions & 1 deletion js/src/components/lineage/lineage.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable @typescript-eslint/no-unnecessary-condition */
// TODO LineageData typing needs to be fully thought out to handle the edge-cases - JMS
import { Node, Edge, Position } from "reactflow";
import { getNeighborSet, union } from "./graph";
import { Run } from "@/lib/api/types";
Expand All @@ -11,7 +13,7 @@ import {
} from "@/lib/api/info";

/**
* THe types for internal data structures.
* The types for internal data structures.
*/

export interface LineageGraphNode {
Expand Down
2 changes: 2 additions & 0 deletions js/src/components/lineage/useMultiNodesAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export const useMultiNodesAction = (
actionState.currentRun = { run_id };
actionState.total = 1;

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
while (true) {
const run = await waitRun(run_id, 2);
actionState.currentRun = run;
Expand Down Expand Up @@ -150,6 +151,7 @@ export const useMultiNodesAction = (
};
onActionNodeUpdated(node);

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
while (true) {
const run = await waitRun(run_id, 2);
actionState.currentRun = run;
Expand Down
13 changes: 10 additions & 3 deletions js/src/components/profile/ProfileDiffForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,22 @@ export function ProfileDiffForm({
<Select
isMulti
closeMenuOnSelect={false}
options={(columnNames || []).map((c) => ({ label: c, value: c }))}
options={columnNames.map((c) => ({ label: c, value: c }))}
value={(params.columns ?? []).map((c) => ({
label: c,
value: c,
}))}
onChange={(options) => {
onChange={(newValue) => {
let cols: string[] | undefined;
const newCols = newValue.map((v) => v.value);
if (newCols.length === 0) {
cols = undefined;
} else {
cols = newCols;
}
onParamsChanged({
...params,
columns: (options || []).map((v) => v.value),
columns: cols,
});
}}></Select>
)}
Expand Down
6 changes: 3 additions & 3 deletions js/src/components/query/QueryResultView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ const PrivateQueryResultView = (
) => {
const pinnedColumns = useMemo(() => viewOptions?.pinned_columns || [], [viewOptions]);

const dataframe = run.result as DataFrame;
const dataframe = run.result;
const gridData = useMemo(() => {
if (!dataframe) {
return { rows: [], columns: [] };
Expand All @@ -149,9 +149,9 @@ const PrivateQueryResultView = (
return <Center height="100%">No data</Center>;
}

const limit = dataframe.limit || 0;
const limit = dataframe ? (dataframe.limit ?? 0) : 0;
const warning =
limit > 0 && dataframe.more
limit > 0 && dataframe?.more
? `Warning: Displayed results are limited to ${limit.toLocaleString()} records. To ensure complete data retrieval, consider applying a LIMIT or WHERE clause to constrain the result set.`
: null;
const showTopBar = onAddToChecklist || warning;
Expand Down
2 changes: 1 addition & 1 deletion js/src/components/run/RunView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const RunView = forwardRef(
<Center p="16px" height="100%" bg="rgb(249,249,249)">
<VStack>
<Flex alignItems="center">
{progress?.percentage === undefined || progress.percentage === null ? (
{progress?.percentage == null ? (
<CircularProgress isIndeterminate size="20px" mr="8px" />
) : (
<CircularProgress size="20px" value={progress.percentage * 100} mr="8px" />
Expand Down
2 changes: 1 addition & 1 deletion js/src/components/schema/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test("column diff", () => {
name: "VARCHAR",
});

const result = mergeColumns(base, current) || {};
const result = mergeColumns(base, current);
expect(Object.keys(result)).toStrictEqual([
"id",
"user_id",
Expand Down
1 change: 1 addition & 0 deletions js/src/components/schema/schemaDiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function isSchemaChanged(

// modified
for (const key of currKeys) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!baseSchema[key] || baseSchema[key].type !== currSchema[key].type) {
return true;
}
Expand Down
3 changes: 2 additions & 1 deletion js/src/components/summary/ChangeSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ function calculateColumnChange(base: NodeData | undefined, current: NodeData | u
// Modify columns
if (current && base) {
Object.keys(current.columns || {}).forEach((col) => {
if (base.columns && current.columns && base.columns[col]) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (base.columns && current.columns?.[col] && base.columns[col]) {
if (base.columns[col].type !== current.columns[col].type) modifies++;
}
});
Expand Down
6 changes: 0 additions & 6 deletions js/src/components/summary/SchemaSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,9 @@ import {
SimpleGrid,
Text,
HStack,
Tooltip,
Tag,
TagLeftIcon,
TagLabel,
SkeletonText,
} from "@chakra-ui/react";
import { LineageGraph, LineageGraphNode } from "../lineage/lineage";
import { SchemaView } from "../schema/SchemaView";
import { mergeColumns } from "../schema/schema";
import { mergeKeysWithStatus } from "@/lib/mergeKeys";
import { useEffect, useState } from "react";
import { ResourceTypeTag, RowCountDiffTag } from "../lineage/NodeTag";
Expand Down
17 changes: 12 additions & 5 deletions js/src/components/valuediff/ValueDiffForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function ValueDiffForm({ params, onParamsChanged, setIsReadyToExecute }:

const columnNames = columns.map((c) => c.name);

// primaryKey can be array or string, map to array
// primaryKey can be an array or string, map to array
const primaryKeys = Array.isArray(primaryKey)
? primaryKey
: primaryKey
Expand All @@ -65,7 +65,7 @@ export function ValueDiffForm({ params, onParamsChanged, setIsReadyToExecute }:
placeholder="Select primary key"
isMulti
closeMenuOnSelect={false}
options={(columnNames || []).map((c) => ({ label: c, value: c }))}
options={columnNames.map((c) => ({ label: c, value: c }))}
value={(primaryKeys || []).map((c) => ({
label: c,
value: c,
Expand Down Expand Up @@ -95,15 +95,22 @@ export function ValueDiffForm({ params, onParamsChanged, setIsReadyToExecute }:
<Select
isMulti
closeMenuOnSelect={false}
options={(columnNames || []).map((c) => ({ label: c, value: c }))}
options={columnNames.map((c) => ({ label: c, value: c }))}
value={(params.columns || []).map((c) => ({
label: c,
value: c,
}))}
onChange={(options) => {
onChange={(newValue) => {
let cols: string[] | undefined;
const newCols = newValue.map((v) => v.value);
if (newCols.length === 0) {
cols = undefined;
} else {
cols = newCols;
}
onParamsChanged({
...params,
columns: (options || []).map((v) => v.value),
columns: cols,
});
}}></Select>
)}
Expand Down
13 changes: 4 additions & 9 deletions js/src/components/valuediff/ValueDiffResultView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,8 @@ function _ValueDiffResultView({ run }: ValueDiffResultViewProp, ref: any) {
const result = run.result as ValueDiffResult;
const params = run.params as ValueDiffParams;
const cellClass = (row: any) => {
const value: number | undefined = row[2];

return value !== undefined && value !== null && value < 1 ? "diff-cell-modified" : "";
const value = row[2] as unknown as number | undefined;
return value != null && value < 1 ? "diff-cell-modified" : "";
};
const primaryKeys = Array.isArray(params.primary_key) ? params.primary_key : [params.primary_key];

Expand Down Expand Up @@ -127,12 +126,8 @@ function _ValueDiffResultView({ run }: ValueDiffResultViewProp, ref: any) {
name: "Matched %",
resizable: true,
renderCell: ({ column, row }) => {
const value: number | undefined = row[column.key];
return (
<Box textAlign="end">
{value != undefined && value !== null ? `${(value * 100).toFixed(2)} %` : "N/A"}
</Box>
);
const value = row[column.key] as unknown as number | undefined;
return <Box textAlign="end">{value != null ? `${(value * 100).toFixed(2)} %` : "N/A"}</Box>;
},
cellClass,
},
Expand Down
2 changes: 2 additions & 0 deletions js/src/components/valuediff/valuediff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ export function toValueDiffGrid(
}

const primaryIndexes = _getPrimaryKeyIndexes(df.columns, primaryKeys);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
const inBaseIndex = (columnMap.in_a || columnMap.IN_A).index;
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
const inCurrentIndex = (columnMap.in_b || columnMap.IN_B).index;

df.data.forEach((row, index) => {
Expand Down
2 changes: 1 addition & 1 deletion js/src/lib/api/profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface TopKDiffParams {
}

export interface TopKResult {
values: (string | number)[];
values: (string | number | undefined)[];
counts: number[];
valids: number;
}
Expand Down
Loading
Loading