Skip to content

Commit a0acc42

Browse files
salazarmgibsondan
authored andcommitted
[Selection input autocomplete] Don't compare constructor names to hardcoded string (#27951)
## Summary & Motivation Turns out the auto-complete suggestions weren't working as expected due to comparing constructor names which get minified in the built app. I will research a better approach than this but for now comparing to the name of the classes works. ## How I Tested These Changes built the app and tested the auto-complete behavior
1 parent 5cc6b44 commit a0acc42

File tree

3 files changed

+54
-19
lines changed

3 files changed

+54
-19
lines changed

js_modules/dagster-ui/packages/ui-core/src/selection/BaseSelectionVisitor.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ import {RuleNode} from 'antlr4ts/tree/RuleNode';
66
import {
77
ParenthesizedExpressionContext,
88
PostAttributeValueWhitespaceContext,
9+
PostExpressionWhitespaceContext,
10+
PostLogicalOperatorWhitespaceContext,
11+
PostNeighborTraversalWhitespaceContext,
12+
PostNotOperatorWhitespaceContext,
13+
PostUpwardTraversalWhitespaceContext,
914
} from './generated/SelectionAutoCompleteParser';
1015
import {SelectionAutoCompleteVisitor as _SelectionAutoCompleteVisitor} from './generated/SelectionAutoCompleteVisitor';
1116

@@ -79,10 +84,9 @@ export class BaseSelectionVisitor
7984
// If child's start..stop doesn't include the cursor, skip
8085
// (unless forced)
8186
if (child.start && child.stop) {
82-
const isWhitespace = child.constructor.name.endsWith('WhitespaceContext');
8387
if (
8488
!this.nodeIncludesCursor(child) &&
85-
(!isWhitespace || child.start.startIndex !== this.cursorIndex) &&
89+
(!isWhitespaceContext(child) || child.start.startIndex !== this.cursorIndex) &&
8690
!this.forceVisitCtx.has(child)
8791
) {
8892
continue;
@@ -91,7 +95,8 @@ export class BaseSelectionVisitor
9195
const nextChild = i + 1 < childCount ? (node.getChild(i + 1) as ParserRuleContext) : null;
9296
if (
9397
!this.nodeIncludesCursor(child, 0) &&
94-
nextChild?.constructor.name.endsWith('WhitespaceContext') &&
98+
nextChild &&
99+
isWhitespaceContext(nextChild) &&
95100
!this.forceVisitCtx.has(child)
96101
) {
97102
continue;
@@ -152,3 +157,17 @@ export class BaseSelectionVisitor
152157

153158
public visitPostExpressionWhitespace(_ctx: any) {}
154159
}
160+
161+
function isWhitespaceContext(ctx: ParserRuleContext) {
162+
switch (ctx.constructor.name) {
163+
case PostAttributeValueWhitespaceContext.name:
164+
case PostExpressionWhitespaceContext.name:
165+
case PostLogicalOperatorWhitespaceContext.name:
166+
case PostNeighborTraversalWhitespaceContext.name:
167+
case PostNotOperatorWhitespaceContext.name:
168+
case PostUpwardTraversalWhitespaceContext.name:
169+
return true;
170+
default:
171+
return false;
172+
}
173+
}

js_modules/dagster-ui/packages/ui-core/src/selection/SelectionAutoCompleteVisitor.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export class SelectionAutoCompleteVisitor extends BaseSelectionVisitor {
180180
this.stopReplacementIndex = ctx.stop!.stopIndex + 1;
181181

182182
const parentChildren = ctx.parent?.children ?? [];
183-
if (parentChildren[0]?.constructor.name === 'AttributeNameContext') {
183+
if (parentChildren[0]?.constructor.name === AttributeNameContext.name) {
184184
const rawValue = getValueNodeValue(ctx.value());
185185
this.addAttributeValueResults(parentChildren[0].text, rawValue);
186186
}
@@ -192,12 +192,12 @@ export class SelectionAutoCompleteVisitor extends BaseSelectionVisitor {
192192
let valueNode: ParserRuleContext | null = null;
193193

194194
const parentChildren = ctx.parent?.children ?? [];
195-
if (parentChildren[0]?.constructor.name === 'AttributeNameContext') {
195+
if (parentChildren[0]?.constructor.name === AttributeNameContext.name) {
196196
attributeName = parentChildren[0] as ParserRuleContext;
197197
}
198-
if (parentChildren[1]?.constructor.name === 'AttributeValueContext') {
198+
if (parentChildren[1]?.constructor.name === AttributeValueContext.name) {
199199
valueNode = parentChildren[1] as ParserRuleContext;
200-
} else if (parentChildren[2]?.constructor.name === 'AttributeValueContext') {
200+
} else if (parentChildren[2]?.constructor.name === AttributeValueContext.name) {
201201
valueNode = parentChildren[2] as ParserRuleContext;
202202
}
203203

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import {ParserRuleContext} from 'antlr4ts';
22

3-
import {AttributeValueContext} from './generated/SelectionAutoCompleteParser';
3+
import {
4+
AttributeValueContext,
5+
IncompleteLeftQuotedStringValueContext,
6+
IncompleteRightQuotedStringValueContext,
7+
QuotedStringValueContext,
8+
UnclosedExpressionlessFunctionExpressionContext,
9+
UnclosedFunctionExpressionContext,
10+
UnclosedParenthesizedExpressionContext,
11+
UnquotedStringValueContext,
12+
} from './generated/SelectionAutoCompleteParser';
413

514
export const removeQuotesFromString = (value: string) => {
615
if (value.length > 1 && value[0] === '"' && value[value.length - 1] === '"') {
@@ -11,28 +20,35 @@ export const removeQuotesFromString = (value: string) => {
1120

1221
export function getValueNodeValue(ctx: ParserRuleContext | null) {
1322
switch (ctx?.constructor.name) {
14-
case 'UnquotedStringValueContext':
23+
case UnquotedStringValueContext.name:
1524
return ctx.text;
16-
case 'IncompleteLeftQuotedStringValueContext':
25+
case IncompleteLeftQuotedStringValueContext.name:
1726
return ctx.text.slice(1);
18-
case 'IncompleteRightQuotedStringValueContext':
27+
case IncompleteRightQuotedStringValueContext.name:
1928
return ctx.text.slice(0, -1);
20-
case 'QuotedStringValueContext':
29+
case QuotedStringValueContext.name:
2130
return ctx.text.slice(1, -1);
22-
case 'AttributeValueContext':
31+
case AttributeValueContext.name:
2332
return getValueNodeValue((ctx as AttributeValueContext).value());
2433
default:
2534
return ctx?.text ?? '';
2635
}
2736
}
2837

29-
export function isInsideExpressionlessParenthesizedExpression(context: ParserRuleContext) {
30-
if (context.parent) {
31-
const nodeType = context.parent.constructor.name;
32-
if (nodeType.startsWith('Unclosed')) {
33-
return true;
38+
export function isInsideExpressionlessParenthesizedExpression(
39+
context: ParserRuleContext | undefined,
40+
) {
41+
const parent = context?.parent;
42+
if (parent) {
43+
const nodeType = parent.constructor.name;
44+
switch (nodeType) {
45+
case UnclosedParenthesizedExpressionContext.name:
46+
case UnclosedFunctionExpressionContext.name:
47+
case UnclosedExpressionlessFunctionExpressionContext.name:
48+
return true;
49+
default:
50+
return isInsideExpressionlessParenthesizedExpression(parent);
3451
}
35-
return isInsideExpressionlessParenthesizedExpression(context.parent);
3652
}
3753
return false;
3854
}

0 commit comments

Comments
 (0)