Skip to content

Commit

Permalink
[Selection Input] Refactor auto-complete system. (dagster-io#27389)
Browse files Browse the repository at this point in the history
## Summary & Motivation

The primary goal of this refactor is to decouple the auto-complete
suggestion UI from the autocomplete visitor logic.

Before:
- Visitor hard coded the types of the suggestions 
- UI rendering was tightly coupled to those types
- The types were too restrictive, they required all of the values to be
strings but that doesn't work well for tags which have a key and a
value. Encoding the tags as a string (eg: `"key"="value"`) created
ambiguities, where we could not tell if the string was a tag of `{key:
"key", value: "value"}` or a tag of `{key: "key"="value", value:
undefined}`.


After:
- Rendering is encapsulated in the SelectionAutoCompleteProvider which
is also responsible for making auto-complete suggestions and can handle
both string and tag object types.



## How I Tested These Changes

Existing jest test in SelectionAutocomplete.ts. + Manual testing in
cloud and OSS.
  • Loading branch information
salazarm authored and LoHertel committed Feb 11, 2025
1 parent bd44bd4 commit 86fa994
Show file tree
Hide file tree
Showing 17 changed files with 1,301 additions and 1,395 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {useCallback, useLayoutEffect, useMemo, useState} from 'react';
import {FeatureFlag} from 'shared/app/FeatureFlags.oss';
import {AssetGraphAssetSelectionInput} from 'shared/asset-graph/AssetGraphAssetSelectionInput.oss';
import {useAssetGraphExplorerFilters} from 'shared/asset-graph/useAssetGraphExplorerFilters.oss';
import {AssetSelectionInput} from 'shared/asset-selection/input/AssetSelectionInput.oss';
import styled from 'styled-components';

import {AssetEdges} from './AssetEdges';
Expand Down Expand Up @@ -53,7 +54,6 @@ import {
import {AssetLocation, useFindAssetLocation} from './useFindAssetLocation';
import {featureEnabled} from '../app/Flags';
import {AssetLiveDataRefreshButton} from '../asset-data/AssetLiveDataProvider';
import {AssetSelectionInput} from '../asset-selection/input/AssetSelectionInput';
import {LaunchAssetExecutionButton} from '../assets/LaunchAssetExecutionButton';
import {AssetKey} from '../assets/types';
import {DEFAULT_MAX_ZOOM, SVGViewport} from '../graph/SVGViewport';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type {Linter} from 'codemirror/addon/lint/lint';
import {useMemo} from 'react';
import {
AssetSelectionLexer,
AssetSelectionParser,
} from 'shared/asset-selection/AssetSelectionAntlr.oss';
import {createUseAssetSelectionAutoComplete as defaultCreateUseAssetSelectionAutoComplete} from 'shared/asset-selection/input/useAssetSelectionAutoComplete.oss';
import {useAssetSelectionAutoCompleteProvider as defaultUseAssetSelectionAutoCompleteProvider} from 'shared/asset-selection/input/useAssetSelectionAutoCompleteProvider.oss';

import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData';
import {SelectionAutoCompleteProvider} from '../../selection/SelectionAutoCompleteProvider';
import {SelectionAutoCompleteInput} from '../../selection/SelectionInput';
import {createSelectionLinter} from '../../selection/createSelectionLinter';

Expand All @@ -15,7 +15,9 @@ interface AssetSelectionInputProps {
value: string;
onChange: (value: string) => void;
linter?: Linter<any>;
createUseAssetSelectionAutoComplete?: typeof defaultCreateUseAssetSelectionAutoComplete;
useAssetSelectionAutoComplete?: (
assets: AssetGraphQueryItem[],
) => Pick<SelectionAutoCompleteProvider, 'useAutoComplete'>;
}

const defaultLinter = createSelectionLinter({
Expand All @@ -28,17 +30,14 @@ export const AssetSelectionInput = ({
onChange,
assets,
linter = defaultLinter,
createUseAssetSelectionAutoComplete = defaultCreateUseAssetSelectionAutoComplete,
useAssetSelectionAutoComplete = defaultUseAssetSelectionAutoCompleteProvider,
}: AssetSelectionInputProps) => {
const useAssetSelectionAutoComplete = useMemo(
() => createUseAssetSelectionAutoComplete(assets),
[assets, createUseAssetSelectionAutoComplete],
);
const {useAutoComplete} = useAssetSelectionAutoComplete(assets);

return (
<SelectionAutoCompleteInput
id="asset-selection-input"
useAutoComplete={useAssetSelectionAutoComplete}
useAutoComplete={useAutoComplete}
placeholder="Search and filter assets"
linter={linter}
value={value}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {useMemo} from 'react';

import {attributeToIcon, getAttributesMap} from './util';
import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData';
import {createSelectionAutoComplete} from '../../selection/SelectionAutoComplete';
import {
SelectionAutoCompleteProvider,
createProvider,
} from '../../selection/SelectionAutoCompleteProvider';

export function useAssetSelectionAutoCompleteProvider(
assets: AssetGraphQueryItem[],
): Pick<SelectionAutoCompleteProvider, 'useAutoComplete'> {
const attributesMap = useMemo(() => getAttributesMap(assets), [assets]);

const baseProvider = useMemo(
() =>
createProvider({
attributesMap,
primaryAttributeKey: 'key',
attributeToIcon,
}),
[attributesMap],
);
const selectionHint = useMemo(() => createSelectionAutoComplete(baseProvider), [baseProvider]);

return {
useAutoComplete: ({line, cursorIndex}) => {
const autoCompleteResults = useMemo(
() => selectionHint(line, cursorIndex),
[line, cursorIndex],
);
return {
autoCompleteResults,
loading: false,
};
},
};
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {FeatureFlag} from 'shared/app/FeatureFlags.oss';
import {AssetGraphAssetSelectionInput} from 'shared/asset-graph/AssetGraphAssetSelectionInput.oss';
import {AssetSelectionInput} from 'shared/asset-selection/input/AssetSelectionInput.oss';
import {useAssetSelectionState} from 'shared/asset-selection/useAssetSelectionState.oss';
import {FilterableAssetDefinition} from 'shared/assets/useAssetDefinitionFilterState.oss';

import {AssetSelectionInput} from './AssetSelectionInput';
import {featureEnabled} from '../../app/Flags';
import {useAssetSelectionFiltering} from '../useAssetSelectionFiltering';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import {IconName} from '@dagster-io/ui-components';

import {assertUnreachable} from '../../app/Util';
import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData';
import {isKindTag} from '../../graph/KindTags';
import {weakMapMemoize} from '../../util/weakMapMemoize';
import {buildRepoPathForHuman} from '../../workspace/buildRepoAddress';

export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
const assetNamesSet: Set<string> = new Set();
const tagNamesSet: Set<string> = new Set();
const tagSet: Set<{key: string; value: string}> = new Set();
const ownersSet: Set<string> = new Set();
const groupsSet: Set<string> = new Set();
const kindsSet: Set<string> = new Set();
Expand All @@ -17,13 +20,7 @@ export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
if (isKindTag(tag)) {
return;
}
if (tag.key && tag.value) {
// We add quotes around the equal sign here because the auto-complete suggestion already wraps the entire value in quotes.
// So wer end up with tag:"key"="value" as the final suggestion
tagNamesSet.add(`${tag.key}"="${tag.value}`);
} else {
tagNamesSet.add(tag.key);
}
tagSet.add(memoizedTag(tag.key, tag.value));
});
asset.node.owners.forEach((owner) => {
switch (owner.__typename) {
Expand Down Expand Up @@ -51,7 +48,7 @@ export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
});

const assetNames = Array.from(assetNamesSet).sort();
const tagNames = Array.from(tagNamesSet).sort();
const tagNames = Array.from(tagSet).sort();
const owners = Array.from(ownersSet).sort();
const groups = Array.from(groupsSet).sort();
const kinds = Array.from(kindsSet).sort();
Expand All @@ -66,3 +63,19 @@ export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
code_location: codeLocations,
};
};

const memoizedTag = weakMapMemoize((key: string, value: string) => ({
key,
value,
}));

export type Attribute = keyof ReturnType<typeof getAttributesMap>;

export const attributeToIcon: Record<Attribute, IconName> = {
key: 'magnify_glass',
kind: 'compute_kind',
code_location: 'code_location',
group: 'asset_group',
owner: 'owner',
tag: 'tag',
};
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import {useMemo} from 'react';
import styled from 'styled-components';

import {RunGraphQueryItem} from './toGraphQueryItems';
import {NO_STATE} from '../run-selection/AntlrRunSelectionVisitor';
import {useGanttChartSelectionAutoCompleteProvider} from './useGanttChartSelectionAutoCompleteProvider';
import {RunSelectionLexer} from '../run-selection/generated/RunSelectionLexer';
import {RunSelectionParser} from '../run-selection/generated/RunSelectionParser';
import {InputDiv, SelectionAutoCompleteInput} from '../selection/SelectionInput';
import {createSelectionLinter} from '../selection/createSelectionLinter';
import {useSelectionInputAutoComplete} from '../selection/useSelectionInputAutoComplete';
import {weakMapMemoize} from '../util/weakMapMemoize';

export const GanttChartSelectionInput = ({
Expand All @@ -19,12 +17,11 @@ export const GanttChartSelectionInput = ({
value: string;
onChange: (value: string) => void;
}) => {
const useAutoComplete = useMemo(() => createAutoComplete(items), [items]);
return (
<Wrapper>
<SelectionAutoCompleteInput
id="run-gantt-chart"
useAutoComplete={useAutoComplete}
useAutoComplete={useGanttChartSelectionAutoCompleteProvider(items).useAutoComplete}
placeholder="Search and filter steps"
linter={getLinter()}
value={value}
Expand All @@ -33,43 +30,10 @@ export const GanttChartSelectionInput = ({
</Wrapper>
);
};

function createAutoComplete(items: RunGraphQueryItem[]) {
return function useAutoComplete(value: string, cursor: number) {
const attributesMap = useMemo(() => {
const statuses = new Set<string>();
const names = new Set<string>();

items.forEach((item) => {
if (item.metadata?.state) {
statuses.add(item.metadata.state);
} else {
statuses.add(NO_STATE);
}
names.add(item.name);
});
return {name: Array.from(names), status: Array.from(statuses)};
}, []);

return {
autoCompleteResults: useSelectionInputAutoComplete({
nameBase: 'name',
attributesMap,
functions: FUNCTIONS,
value,
cursor,
}),
loading: false,
};
};
}

const getLinter = weakMapMemoize(() =>
createSelectionLinter({Lexer: RunSelectionLexer, Parser: RunSelectionParser}),
);

const FUNCTIONS = ['sinks', 'roots'];

const Wrapper = styled.div`
${InputDiv} {
width: 24vw;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import {useMemo} from 'react';

import {RunGraphQueryItem} from './toGraphQueryItems';
import {NO_STATE} from '../run-selection/AntlrRunSelectionVisitor';
import {createSelectionAutoComplete} from '../selection/SelectionAutoComplete';
import {
SelectionAutoCompleteProvider,
createProvider,
} from '../selection/SelectionAutoCompleteProvider';

export function useGanttChartSelectionAutoCompleteProvider(
items: RunGraphQueryItem[],
): Pick<SelectionAutoCompleteProvider, 'useAutoComplete'> {
const attributesMap = useMemo(() => {
const statuses = new Set<string>();
const names = new Set<string>();

items.forEach((item) => {
if (item.metadata?.state) {
statuses.add(item.metadata.state);
} else {
statuses.add(NO_STATE);
}
names.add(item.name);
});
return {name: Array.from(names), status: Array.from(statuses)};
}, [items]);

const baseProvider = useMemo(
() =>
createProvider({
attributesMap,
primaryAttributeKey: 'name',
attributeToIcon: iconMap,
}),
[attributesMap],
);
const selectionHint = useMemo(() => createSelectionAutoComplete(baseProvider), [baseProvider]);

return {
useAutoComplete: ({line, cursorIndex}) => {
const autoCompleteResults = useMemo(
() => selectionHint(line, cursorIndex),
[line, cursorIndex],
);
return {
autoCompleteResults,
loading: false,
};
},
};
}

export const iconMap = {
name: 'magnify_glass',
status: 'status',
} as const;
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import {useMemo} from 'react';
import styled from 'styled-components';

import {useOpGraphSelectionAutoCompleteProvider} from './useOpGraphSelectionAutoCompleteProvider';
import {GraphQueryItem} from '../app/GraphQueryImpl';
import {OpSelectionLexer} from '../op-selection/generated/OpSelectionLexer';
import {OpSelectionParser} from '../op-selection/generated/OpSelectionParser';
import {InputDiv, SelectionAutoCompleteInput} from '../selection/SelectionInput';
import {createSelectionLinter} from '../selection/createSelectionLinter';
import {useSelectionInputAutoComplete} from '../selection/useSelectionInputAutoComplete';
import {weakMapMemoize} from '../util/weakMapMemoize';

export const OpGraphSelectionInput = ({
Expand All @@ -18,12 +17,11 @@ export const OpGraphSelectionInput = ({
value: string;
onChange: (value: string) => void;
}) => {
const useAutoComplete = useMemo(() => createAutoComplete(items), [items]);
return (
<Wrapper>
<SelectionAutoCompleteInput
id="op-graph"
useAutoComplete={useAutoComplete}
useAutoComplete={useOpGraphSelectionAutoCompleteProvider(items).useAutoComplete}
placeholder="Search and filter ops"
linter={getLinter()}
value={value}
Expand All @@ -33,35 +31,10 @@ export const OpGraphSelectionInput = ({
);
};

function createAutoComplete(items: GraphQueryItem[]) {
return function useAutoComplete(value: string, cursor: number) {
const attributesMap = useMemo(() => {
const names = new Set<string>();
items.forEach((item) => {
names.add(item.name);
});
return {name: Array.from(names)};
}, []);

return {
autoCompleteResults: useSelectionInputAutoComplete({
nameBase: 'name',
attributesMap,
functions: FUNCTIONS,
value,
cursor,
}),
loading: false,
};
};
}

const getLinter = weakMapMemoize(() =>
createSelectionLinter({Lexer: OpSelectionLexer, Parser: OpSelectionParser}),
);

const FUNCTIONS = ['sinks', 'roots'];

const Wrapper = styled.div`
${InputDiv} {
width: 24vw;
Expand Down
Loading

0 comments on commit 86fa994

Please sign in to comment.