Skip to content

Commit 35d82c0

Browse files
committed
[ui] Improve typechecking on qs usage
1 parent 5b44544 commit 35d82c0

30 files changed

+242
-113
lines changed

js_modules/dagster-ui/packages/ui-components/src/components/TokenizingField.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ interface ActiveSuggestionInfo {
2828
idx: number;
2929
}
3030

31-
export interface TokenizingFieldValue {
31+
export type TokenizingFieldValue = {
3232
token?: string;
3333
value: string;
34-
}
34+
};
3535

3636
interface TokenizingFieldProps {
3737
values: TokenizingFieldValue[];

js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetGraphExplorer.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,12 @@ const AssetGraphExplorerWithData = ({
266266
const [expandedGroups, setExpandedGroups] = useQueryAndLocalStoragePersistedState<string[]>({
267267
localStorageKey: `asset-graph-open-graph-nodes-${viewType}-${explorerPath.pipelineName}`,
268268
encode: (arr) => ({expanded: arr.length ? arr.join(',') : undefined}),
269-
decode: (qs) => (qs.expanded || '').split(',').filter(Boolean),
269+
decode: (qs) => {
270+
if (typeof qs.expanded === 'string') {
271+
return qs.expanded.split(',').filter(Boolean);
272+
}
273+
return [];
274+
},
270275
isEmptyState: (val) => val.length === 0,
271276
});
272277
const focusGroupIdAfterLayoutRef = React.useRef('');

js_modules/dagster-ui/packages/ui-core/src/asset-graph/sidebar/Sidebar.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@ export const AssetGraphExplorerSidebar = React.memo(
102102
return {'open-nodes': Array.from(val)};
103103
},
104104
decode: (qs) => {
105-
return new Set(qs['open-nodes']);
105+
const openNodes = qs['open-nodes'];
106+
if (Array.isArray(openNodes)) {
107+
return new Set(openNodes.map((node) => String(node)));
108+
}
109+
return new Set();
106110
},
107111
isEmptyState: (val) => val.size === 0,
108112
});

js_modules/dagster-ui/packages/ui-core/src/assets/AllIndividualEventsButton.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,10 +311,10 @@ export const AllIndividualEventsButton = ({
311311
children: React.ReactNode;
312312
disabled?: boolean;
313313
}) => {
314-
const [_open, setOpen] = useQueryPersistedState({
314+
const [_open, setOpen] = useQueryPersistedState<boolean>({
315315
queryKey: 'showAllEvents',
316-
decode: (qs) => (qs.showAllEvents === 'true' ? true : false),
317-
encode: (b) => ({showAllEvents: b || undefined}),
316+
decode: (qs) => typeof qs.showAllEvents === 'string' && qs.showAllEvents === 'true',
317+
encode: (b) => ({showAllEvents: b ? 'true' : undefined}),
318318
});
319319
const [focusedTimestamp, setFocusedTimestamp] = React.useState<string | undefined>();
320320
const groups = React.useMemo(

js_modules/dagster-ui/packages/ui-core/src/assets/AssetPartitions.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ interface Props {
5252
isLoadingDefinition: boolean;
5353
}
5454

55-
const DISPLAYED_STATUSES = [
55+
const DISPLAYED_STATUSES: AssetPartitionStatus[] = [
5656
AssetPartitionStatus.MISSING,
5757
AssetPartitionStatus.MATERIALIZING,
5858
AssetPartitionStatus.MATERIALIZED,
@@ -88,10 +88,17 @@ export const AssetPartitions = ({
8888
const [statusFilters, setStatusFilters] = useQueryPersistedState<AssetPartitionStatus[]>({
8989
defaults: {status: [...DISPLAYED_STATUSES].sort().join(',')},
9090
encode: (val) => ({status: [...val].sort().join(',')}),
91-
decode: (qs) =>
92-
(qs.status || '')
93-
.split(',')
94-
.filter((s: AssetPartitionStatus) => DISPLAYED_STATUSES.includes(s)),
91+
decode: (qs) => {
92+
const status = qs.status;
93+
if (typeof status === 'string') {
94+
return status
95+
.split(',')
96+
.filter((s): s is AssetPartitionStatus =>
97+
DISPLAYED_STATUSES.includes(s as AssetPartitionStatus),
98+
);
99+
}
100+
return [];
101+
},
95102
});
96103

97104
const [searchValues, setSearchValues] = useState<string[]>([]);

js_modules/dagster-ui/packages/ui-core/src/assets/AutoMaterializePolicyPage/AssetAutomaterializePolicyPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const AssetAutomaterializePolicyPage = ({
3333
>({
3434
queryKey: 'evaluation',
3535
decode: (raw) => {
36-
return raw.evaluation;
36+
return typeof raw.evaluation === 'string' ? raw.evaluation : undefined;
3737
},
3838
encode: (raw) => {
3939
// Reset the selected partition

js_modules/dagster-ui/packages/ui-core/src/assets/types.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export type AssetDefinition = Extract<
1111

1212
export type AssetLineageScope = 'neighbors' | 'upstream' | 'downstream';
1313

14-
export interface AssetViewParams {
14+
export type AssetViewParams = {
1515
view?: string;
1616
lineageScope?: AssetLineageScope;
1717
lineageDepth?: number;
@@ -23,4 +23,4 @@ export interface AssetViewParams {
2323
default_range?: string;
2424
column?: string;
2525
showAllEvents?: boolean;
26-
}
26+
};

js_modules/dagster-ui/packages/ui-core/src/assets/useAssetDefinitionFilterState.oss.tsx

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
import {useQueryPersistedState} from '../hooks/useQueryPersistedState';
1313
import {doesFilterArrayMatchValueArray} from '../ui/Filters/doesFilterArrayMatchValueArray';
1414
import {Tag} from '../ui/Filters/useDefinitionTagFilter';
15-
import {buildRepoAddress} from '../workspace/buildRepoAddress';
1615
import {RepoAddress} from '../workspace/types';
1716

1817
type Nullable<T> = {
@@ -42,6 +41,18 @@ export type AssetFilterType = AssetFilterBaseType & {
4241
selectAllFilters: Array<keyof AssetFilterBaseType>;
4342
};
4443

44+
const parseJSONArraySafely = (value: qs.ParsedQs[string] | undefined) => {
45+
if (typeof value === 'string') {
46+
try {
47+
const parsed = JSON.parse(value);
48+
if (Array.isArray(parsed)) {
49+
return parsed;
50+
}
51+
} catch {}
52+
}
53+
return [];
54+
};
55+
4556
export const useAssetDefinitionFilterState = ({isEnabled = true}: {isEnabled?: boolean}) => {
4657
const [filters, setFilters] = useQueryPersistedState<AssetFilterType>({
4758
encode: isEnabled
@@ -55,20 +66,19 @@ export const useAssetDefinitionFilterState = ({isEnabled = true}: {isEnabled?: b
5566
selectAllFilters: selectAllFilters?.length ? JSON.stringify(selectAllFilters) : undefined,
5667
})
5768
: () => ({}),
58-
decode: (qs) => ({
59-
groups: qs.groups && isEnabled ? JSON.parse(qs.groups) : [],
60-
kinds: qs.kinds && isEnabled ? JSON.parse(qs.kinds) : [],
61-
changedInBranch: qs.changedInBranch && isEnabled ? JSON.parse(qs.changedInBranch) : [],
62-
owners: qs.owners && isEnabled ? JSON.parse(qs.owners) : [],
63-
tags: qs.tags && isEnabled ? JSON.parse(qs.tags) : [],
64-
codeLocations:
65-
qs.codeLocations && isEnabled
66-
? JSON.parse(qs.codeLocations).map((repo: RepoAddress) =>
67-
buildRepoAddress(repo.name, repo.location),
68-
)
69-
: [],
70-
selectAllFilters: qs.selectAllFilters ? JSON.parse(qs.selectAllFilters) : [],
71-
}),
69+
decode: (qs) => {
70+
// JSON will be parsed safely, but we are not verifying the types within the arrays themselves
71+
// so there could be downstream issues if the arrays are of incorrect types.
72+
return {
73+
groups: isEnabled ? parseJSONArraySafely(qs.groups) : [],
74+
kinds: isEnabled ? parseJSONArraySafely(qs.kinds) : [],
75+
changedInBranch: isEnabled ? parseJSONArraySafely(qs.changedInBranch) : [],
76+
owners: isEnabled ? parseJSONArraySafely(qs.owners) : [],
77+
tags: isEnabled ? parseJSONArraySafely(qs.tags) : [],
78+
codeLocations: isEnabled ? parseJSONArraySafely(qs.codeLocations) : [],
79+
selectAllFilters: isEnabled ? parseJSONArraySafely(qs.selectAllFilters) : [],
80+
};
81+
},
7282
});
7383

7484
const filterFn = useCallback(

js_modules/dagster-ui/packages/ui-core/src/assets/useAssetEventsFilters.tsx

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,30 @@ export const useAssetEventsFilters = ({assetKey, assetNode}: Config) => {
4242
type: typeValues.map((t) => t.key),
4343
};
4444
}
45+
46+
let dateRange: {start: number | null; end: number | null} | undefined;
47+
if (raw?.dateRange && typeof raw.dateRange !== 'string' && !Array.isArray(raw.dateRange)) {
48+
dateRange = {
49+
start: typeof raw.dateRange.start === 'string' ? parseInt(raw.dateRange.start) : null,
50+
end: typeof raw.dateRange.end === 'string' ? parseInt(raw.dateRange.end) : null,
51+
};
52+
}
53+
4554
return {
46-
partitions: raw?.partitions,
47-
dateRange: raw?.dateRange
48-
? {
49-
start: raw.dateRange.start ? parseInt(raw.dateRange.start) : null,
50-
end: raw.dateRange.end ? parseInt(raw.dateRange.end) : null,
51-
}
52-
: undefined,
53-
status: raw?.status,
54-
type: raw?.type,
55+
partitions: Array.isArray(raw?.partitions) ? raw.partitions.map(String) : [],
56+
dateRange,
57+
status: Array.isArray(raw?.status) ? raw.status.map(String) : [],
58+
type: Array.isArray(raw?.type) ? raw.type.map(String) : [],
5559
};
5660
},
5761
encode: (raw) => ({
5862
partitions: raw.partitions,
59-
dateRange: raw.dateRange,
63+
dateRange: raw.dateRange
64+
? {
65+
start: raw.dateRange.start ? String(raw.dateRange.start) : undefined,
66+
end: raw.dateRange.end ? String(raw.dateRange.end) : undefined,
67+
}
68+
: undefined,
6069
status: raw.status,
6170
type: raw.type,
6271
}),

js_modules/dagster-ui/packages/ui-core/src/assets/useAssetViewParams.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1+
import qs from 'qs';
2+
13
import {AssetViewParams} from './types';
24
import {useQueryPersistedState} from '../hooks/useQueryPersistedState';
35

4-
export const decode = ({lineageDepth, showAllEvents, ...rest}: {[key: string]: any}) => {
6+
export const decode = ({lineageDepth, showAllEvents, ...rest}: qs.ParsedQs) => {
57
const result: AssetViewParams = {...rest};
6-
if (lineageDepth !== undefined) {
8+
if (typeof lineageDepth === 'string') {
79
result.lineageDepth = Number(lineageDepth);
810
}
9-
if (showAllEvents !== undefined) {
11+
if (typeof showAllEvents === 'string') {
1012
result.showAllEvents =
1113
showAllEvents === 'true' ? true : showAllEvents === 'false' ? false : undefined;
1214
}

0 commit comments

Comments
 (0)