Skip to content

Commit

Permalink
[ui] Update eslint and TS in most of the dagster-ui workspace (#25317)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Update TS and run lint in app-oss and ui-core, picking up fixes that are
surfaced by the recent change to our eslint-config.

ui-components will be done in a followup, since it references the
published eslint-config package.

## How I Tested These Changes

`make ts`, `make lint` in dagster-ui.
  • Loading branch information
hellendag authored Oct 16, 2024
1 parent bb898b8 commit cbe2e1e
Show file tree
Hide file tree
Showing 229 changed files with 1,651 additions and 1,712 deletions.
8 changes: 6 additions & 2 deletions js_modules/dagster-ui/packages/app-oss/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@
"@types/uuid": "^8.3.4",
"@types/validator": "^13",
"@types/webpack-bundle-analyzer": "^4",
"@typescript-eslint/eslint-plugin": "^8.9.0",
"@typescript-eslint/parser": "^8.9.0",
"eslint": "^8.57.0",
"eslint-plugin-import": "^2.31.0",
"eslint-plugin-jest": "^26.4.6",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-unused-imports": "^4.1.4",
"eslint-webpack-plugin": "3.1.1",
"file-loader": "^6.2.0",
"prettier": "^3.0.3",
"typescript": "5.4.5",
"prettier": "^3.3.3",
"typescript": "5.5.4",
"webpack": "^5.94.0",
"webpack-bundle-analyzer": "^4.7.0",
"webpack-stats-plugin": "^1.1.3"
Expand Down
3 changes: 2 additions & 1 deletion js_modules/dagster-ui/packages/eslint-config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ module.exports = {
'react/prop-types': 'off',
'react/display-name': 'off',
'react/react-in-jsx-scope': 'off',
'@typescript-eslint/ban-types': [
'@typescript-eslint/no-restricted-types': [
'error',
{
types: {
Expand All @@ -139,6 +139,7 @@ module.exports = {
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/prefer-interface': 'off',
'@typescript-eslint/no-empty-interface': 'off',
'@typescript-eslint/no-empty-object-type': ['error', {allowInterfaces: 'with-single-extends'}],
'react-hooks/rules-of-hooks': 'error',
'react-hooks/exhaustive-deps': 'warn',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ const renderTypeRecursive: renderTypeRecursiveType = (type, typeLookup, depth, p
export const tryPrettyPrintJSON = (jsonString: string) => {
try {
return JSON.stringify(JSON.parse(jsonString), null, 2);
} catch (err) {
} catch {
// welp, looks like it's not valid JSON. This has happened at least once
// in the wild - a user was able to build a metadata entry in Python with
// a `NaN` number value: https://github.com/dagster-io/dagster/issues/8959
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ export const useCountdown = (config: Config) => {
const token = React.useRef<ReturnType<typeof setInterval> | null>(null);

const clearToken = React.useCallback(() => {
token.current && clearInterval(token.current);
if (token.current) {
clearInterval(token.current);
}
token.current = null;
}, []);

Expand All @@ -49,7 +51,9 @@ export const useCountdown = (config: Config) => {
React.useEffect(() => {
if (remainingTime === 0) {
clearToken();
onComplete && onComplete();
if (onComplete) {
onComplete();
}
}
}, [clearToken, onComplete, remainingTime]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ export const TokenizingField = ({

const _onTextChange = (text: string) => {
setTyped(text);
onTextChange && onTextChange(text);
if (onTextChange) {
onTextChange(text);
}
};

// We need to manage selection in the dropdown by ourselves. To ensure the
Expand Down Expand Up @@ -422,7 +424,9 @@ export const TokenizingField = ({
inputProps={{
onFocus: () => {
setOpen(true);
onFocus && onFocus();
if (onFocus) {
onFocus();
}
},
onBlur: () => {
// Emulate behavior of addOnBlur for TagInput
Expand All @@ -431,7 +435,9 @@ export const TokenizingField = ({
onConfirmText(typed);
}
setOpen(false);
onBlur && onBlur();
if (onBlur) {
onBlur();
}
},
}}
$maxWidth={fullwidth ? '100%' : undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export const Tooltip = (props: Props) => {
document.body.addEventListener('mousemove', listener);
}
return () => {
listener && document.body.removeEventListener('mousemove', listener);
if (listener) {
document.body.removeEventListener('mousemove', listener);
}
};
}, [isOpen, useDisabledButtonTooltipFix]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ export const Multiple = () => {
const onClick = useCallback((id: string) => {
setActiveItems((current) => {
const copy = new Set(current);
copy.has(id) ? copy.delete(id) : copy.add(id);
if (copy.has(id)) {
copy.delete(id);
} else {
copy.add(id);
}
return copy;
});
}, []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ export const Default = () => {
<NonIdealState
icon="error"
title="Query Error"
description={
"This is an example error message, in reality they're probably longer than this."
}
description="This is an example error message, in reality they're probably longer than this."
/>
</Group>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ export const Sizes = () => {
)}
onChange={(values: number[]) => {
const [first] = values;
first && setValue(first);
if (typeof first === 'number') {
setValue(first);
}
}}
>
<MultiSlider.Handle value={value} type="full" intentAfter={Intent.PRIMARY} />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {memo} from 'react';
import styled from 'styled-components';

import {ConfigEditorHelpContext} from './types/ConfigEditorHelpContext';
import {Colors} from '../Color';
import {ConfigTypeSchema, TypeData} from '../ConfigTypeSchema';
import {ConfigEditorHelpContext} from './types/ConfigEditorHelpContext';
import {isHelpContextEqual} from '../configeditor/isHelpContextEqual';

interface ConfigEditorHelpProps {
Expand Down
8 changes: 6 additions & 2 deletions js_modules/dagster-ui/packages/ui-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,18 @@
"@types/styled-components": "^5.1.26",
"@types/testing-library__jest-dom": "^5.14.2",
"@types/ws": "^6.0.3",
"@typescript-eslint/eslint-plugin": "^8.9.0",
"@typescript-eslint/parser": "^8.9.0",
"babel-jest": "^29.7.0",
"babel-loader": "9.1.2",
"babel-plugin-graphql-tag": "^3.3.0",
"babel-plugin-macros": "^2.8.0",
"child-process": "^1.0.2",
"eslint": "^8.57.0",
"eslint-plugin-import": "^2.31.0",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-storybook": "^0.8.0",
"eslint-plugin-unused-imports": "^4.1.4",
"faker": "5.5.3",
"graphql-codegen-typescript-mock-data": "^3.7.1",
"graphql-config": "^5.0.3",
Expand All @@ -141,7 +145,7 @@
"jest": "^29.7.0",
"jest-canvas-mock": "^2.4.0",
"jest-environment-jsdom": "^29.4",
"prettier": "^3.0.3",
"prettier": "^3.3.3",
"react": "^18.3.1",
"react-docgen-typescript-plugin": "^1.0.8",
"react-dom": "^18.3.1",
Expand All @@ -153,7 +157,7 @@
"styled-components": "^5.3.3",
"ts-node": "9.1.1",
"ts-prune": "0.10.3",
"typescript": "5.4.5",
"typescript": "5.5.4",
"webpack": "^5.94.0"
},
"babelMacros": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {patchCopyToRemoveZeroWidthUnderscores} from './Util';
import {WebSocketProvider} from './WebSocketProvider';
import {AnalyticsContext, dummyAnalytics} from './analytics';
import {migrateLocalStorageKeys} from './migrateLocalStorageKeys';
import {TimeProvider} from './time/TimeContext';
import {
ApolloClient,
ApolloLink,
Expand All @@ -30,6 +29,7 @@ import {
InMemoryCache,
split,
} from '../apollo-client';
import {TimeProvider} from './time/TimeContext';
import {AssetLiveDataProvider} from '../asset-data/AssetLiveDataProvider';
import {AssetRunLogObserver} from '../asset-graph/AssetRunLogObserver';
import {CodeLinkProtocolProvider} from '../code-links/CodeLinkProtocol';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,14 @@ export const AppTopNavLogo = () => {
const navButton = React.useRef<null | HTMLButtonElement>(null);

const onToggle = React.useCallback(() => {
navButton.current && navButton.current.focus();
nav.isOpen ? nav.close() : nav.open();
if (navButton.current) {
navButton.current.focus();
}
if (nav.isOpen) {
nav.close();
} else {
nav.open();
}
}, [nav]);

const onKeyDown = React.useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const ConfirmationDialog = ({
onClose,
}: ConfirmationDialogProps) => {
return (
<Dialog icon={title ? icon ?? 'info' : icon} onClose={onClose} title={title} isOpen={open}>
<Dialog icon={title ? (icon ?? 'info') : icon} onClose={onClose} title={title} isOpen={open}>
<DialogBody>{description}</DialogBody>
<DialogFooter topBorder>
<Button onClick={onClose}>Cancel</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export const allStoredSessions = () => {
// If it's not a parseable object, it's not a launchpad session.
try {
parsed = JSON.parse(value);
} catch (e) {
} catch {
continue;
}

Expand Down Expand Up @@ -351,7 +351,7 @@ export const writeLaunchpadSessionToStorage =
try {
setState(data);
return true;
} catch (e) {
} catch {
// The data could not be written to localStorage. This is probably due to
// a QuotaExceededError, but since different browsers use slightly different
// objects for this, we don't try to get clever detecting it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ class GraphTraverser<T extends GraphQueryItem> {
item.inputs.forEach((input) =>
input.dependsOn.forEach((d) => {
const item = this.itemNamed(d.solid.name);
item && callback(item);
if (item) {
callback(item);
}
}),
);

Expand All @@ -70,7 +72,9 @@ class GraphTraverser<T extends GraphQueryItem> {
item.outputs.forEach((output) =>
output.dependedBy.forEach((d) => {
const item = this.itemNamed(d.solid.name);
item && callback(item);
if (item) {
callback(item);
}
}),
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import * as React from 'react';

import {gql, useQuery} from '../apollo-client';
import {
PermissionFragment,
PermissionsQuery,
PermissionsQueryVariables,
} from './types/Permissions.types';
import {gql, useQuery} from '../apollo-client';

// used in tests, to ensure against permission renames. Should make sure that the mapping in
// extractPermissions is handled correctly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import styled from 'styled-components';

import {showSharedToaster} from './DomUtils';
import {useCopyToClipboard} from './browser';
import {PythonErrorChainFragment, PythonErrorFragment} from './types/PythonErrorFragment.types';
import {gql} from '../apollo-client';
import {PythonErrorChainFragment, PythonErrorFragment} from './types/PythonErrorFragment.types';
import {ErrorSource} from '../graphql/types';
import {MetadataEntries} from '../metadata/MetadataEntry';
import {MetadataEntryFragment} from '../metadata/types/MetadataEntryFragment.types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ const UserSettingsDialogContent = ({onClose, visibleFlags}: DialogContentProps)
onClick={() => {
indexedDB.databases().then((databases) => {
databases.forEach((db) => {
db.name && indexedDB.deleteDatabase(db.name);
if (db.name) {
indexedDB.deleteDatabase(db.name);
}
});
});
showCustomAlert({
Expand Down
4 changes: 1 addition & 3 deletions js_modules/dagster-ui/packages/ui-core/src/app/Util.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export function indexedDBAsyncMemoize<T, R, U extends (arg: T, ...rest: any[]) =
dbName: 'indexDBAsyncMemoizeDB',
maxCount: 50,
});
} catch (e) {}
} catch {}

async function genHashKey(arg: T, ...rest: any[]) {
const hash = hashFn ? hashFn(arg, ...rest) : arg;
Expand Down Expand Up @@ -211,8 +211,6 @@ export function indexedDBAsyncMemoize<T, R, U extends (arg: T, ...rest: any[]) =
//
// Uses WeakMap to tie the lifecycle of the cache to the lifecycle of the
// object argument.
//
// eslint-disable-next-line @typescript-eslint/ban-types
export function weakmapMemoize<T extends object, R>(
fn: (arg: T, ...rest: any[]) => R,
): (arg: T, ...rest: any[]) => R {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export const WebSocketProvider = (props: Props) => {
websocketClient.status === WebSocket.OPEN
? 'available'
: websocketClient.status === WebSocket.CLOSED
? 'unavailable'
: 'attempting-to-connect',
? 'unavailable'
: 'attempting-to-connect',
);

const value = React.useMemo(
Expand Down Expand Up @@ -117,7 +117,9 @@ export const WebSocketProvider = (props: Props) => {
}

return () => {
timeout && clearTimeout(timeout);
if (timeout) {
clearTimeout(timeout);
}
};
}, [availability]);

Expand Down
4 changes: 3 additions & 1 deletion js_modules/dagster-ui/packages/ui-core/src/app/browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export const useCopyToClipboard = () => {
}

return () => {
node.current && document.body.removeChild(node.current);
if (node.current) {
document.body.removeChild(node.current);
}
};
}, [clipboardAPI]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const migrateLocalStorageKeys = ({from, to, deleteExisting = false}: Inpu
if (window.localStorage.getItem(newKey) === null) {
try {
window.localStorage.setItem(newKey, value);
} catch (e) {
} catch {
// Failed to write. Probably a QuotaExceededError.
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const TimezoneSelect = ({trigger}: Props) => {
// Some browsers include UTC. (Firefox) Some don't. (Chrome, Safari)
// Determine whether we need to explicitly add it to the list.
explicitlyAddUTC = !allTimezoneItems.some((tz) => tz.key === 'UTC');
} catch (e) {
} catch {
// `Intl.supportedValuesOf` is unavailable in this browser. Only
// support the Automatic timezone and UTC.
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react';

import {ApolloClient, gql, useApolloClient} from '../apollo-client';
import {
AssetGraphLiveQuery,
AssetGraphLiveQueryVariables,
} from './types/AssetBaseDataProvider.types';
import {ApolloClient, gql, useApolloClient} from '../apollo-client';
import {
LiveDataForNode,
buildLiveDataForNode,
Expand Down
Loading

2 comments on commit cbe2e1e

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-2uz5ifx7g-elementl.vercel.app

Built with commit cbe2e1e.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-akloj3laa-elementl.vercel.app

Built with commit cbe2e1e.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.