Skip to content

Commit 66e51b2

Browse files
johallarjohallar
authored
chore: Fix eslint/prettier errors, add checks to CI (#26520)
## Description <!---Describe your changes in detail--> This is a follow on to adding eslint + prettier packages and configs here: #26281 This is a big QOL and code quality improvement for UI development. Formatting and linting will be consistent and enforced across the UI codebase. ## Motivation and Context <!---Why is this change required? What problem does it solve?--> <!---If it fixes an open issue, please link to the issue here.--> This keeps formatting and lint rules consistent across the whole UI codebase, adds steps to the CI to lint + check formatting ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> This should have no user facing impacts ## Test Plan <!---Please fill in how you tested your change--> 1. CI Passes 2. ESLint errors + bugs requiring manual intervention I will call out inline in the PR. These should be tested manually. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes ``` == NO RELEASE NOTE == ```
1 parent b693fac commit 66e51b2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+4789
-3700
lines changed

‎presto-ui/bin/check_webui.sh‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,10 @@ if ! yarn --cwd ${WEBUI_ROOT}/ run flow; then
3535
echo "ERROR: Flow found type errors while performing static analysis"
3636
exit 1
3737
fi
38+
39+
# Fail on eslint errors
40+
41+
if ! yarn --cwd ${WEBUI_ROOT}/ run lint --quiet; then
42+
echo "ERROR: ESlint errors found"
43+
exit 1
44+
fi

‎presto-ui/src/.prettierignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ package-lock.json
66
node_modules/
77
target/
88
static/
9-
9+
sql-parser/

‎presto-ui/src/.prettierrc.json‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,10 @@
66
"printWidth": 120,
77
"overrides": [
88
{
9-
"files": [
10-
"*.js",
11-
"*.jsx"
12-
],
9+
"files": ["*.js", "*.jsx"],
1310
"options": {
1411
"parser": "flow"
1512
}
1613
}
1714
]
18-
}
15+
}

‎presto-ui/src/components/ClusterHUD.jsx‎

Lines changed: 169 additions & 55 deletions
Large diffs are not rendered by default.

‎presto-ui/src/components/LivePlan.jsx‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ function getStages(queryInfo: QueryInfo): Map<string, StageNodeInfo> {
5353
}
5454

5555
function flattenStage(stageInfo: OutputStage, result: any) {
56-
stageInfo.subStages.forEach(function(stage) {
56+
stageInfo.subStages.forEach(function (stage) {
5757
flattenStage(stage, result);
5858
});
5959

@@ -77,11 +77,11 @@ function flattenNode(stages: any, rootNodeInfo: any, node: any, result: Map<any,
7777
name: node["name"],
7878
identifier: node["identifier"],
7979
details: node["details"],
80-
sources: node.children.map(node => node.id),
80+
sources: node.children.map((node) => node.id),
8181
remoteSources: node.remoteSources,
8282
});
8383

84-
node.children.forEach(function(child) {
84+
node.children.forEach(function (child) {
8585
flattenNode(stages, rootNodeInfo, child, result);
8686
});
8787
}
@@ -128,7 +128,6 @@ type PlanNodeProps = {
128128
sources: string[],
129129
remoteSources: string[],
130130
};
131-
type PlanNodeState = {};
132131

133132
export const PlanNode = (props: PlanNodeProps): React.Node => {
134133
return (
@@ -173,9 +172,9 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
173172
const refreshLoop: () => void = useCallback(() => {
174173
clearTimeout(timeoutId.current); // to stop multiple series of refreshLoop from going on simultaneously
175174
fetch("/v1/query/" + props.queryId)
176-
.then(response => response.json())
177-
.then(query => {
178-
setState(prevState => {
175+
.then((response) => response.json())
176+
.then((query) => {
177+
setState((prevState) => {
179178
const ended = query.finalQueryInfo;
180179
if (ended) {
181180
clearTimeout(timeoutId.current);
@@ -191,7 +190,7 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
191190
});
192191
})
193192
.catch(() => {
194-
setState(prevState => ({
193+
setState((prevState) => ({
195194
...prevState,
196195
initialized: true,
197196
}));
@@ -217,21 +216,21 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
217216
graph.setParent(stageRootNodeId, clusterId);
218217
graph.setEdge("node-" + stage.root, stageRootNodeId, { style: "visibility: hidden" });
219218

220-
stage.nodes.forEach(node => {
219+
stage.nodes.forEach((node) => {
221220
const nodeId = "node-" + node.id;
222221
const nodeHtml = ReactDOMServer.renderToString(<PlanNode {...node} />);
223222

224223
graph.setNode(nodeId, { label: nodeHtml, style: "fill: #fff", labelType: "html", class: "text-center" });
225224
graph.setParent(nodeId, clusterId);
226225

227-
node.sources.forEach(source => {
226+
node.sources.forEach((source) => {
228227
graph.setEdge("node-" + source, nodeId, { class: "plan-edge", arrowheadClass: "plan-arrowhead" });
229228
});
230229

231230
if (node.remoteSources.length > 0) {
232231
graph.setNode(nodeId, { label: "", shape: "circle", class: "text-center" });
233232

234-
node.remoteSources.forEach(sourceId => {
233+
node.remoteSources.forEach((sourceId) => {
235234
const source = allStages.get(sourceId);
236235
if (source) {
237236
const sourceStats = source.stageStats;
@@ -263,7 +262,7 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
263262
const svg = initializeSvg(currentSvg);
264263
const graph = graphRef.current;
265264
const stages = getStages(queryInfo);
266-
stages.forEach(stage => {
265+
stages.forEach((stage) => {
267266
updateD3Stage(stage, graph, stages);
268267
});
269268

@@ -296,7 +295,7 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
296295
const zoom = d3
297296
.zoom()
298297
.scaleExtent([initialScale, 1])
299-
.on("zoom", event => {
298+
.on("zoom", (event) => {
300299
inner.attr("transform", event.transform);
301300
});
302301

@@ -352,17 +351,18 @@ export const LivePlan = (props: LivePlanProps): React.Node => {
352351
<div className="loader">Loading...</div>
353352
</div>
354353
</div>
355-
)
354+
);
356355
}
357356
return (
358357
<div>
359-
{!props.isEmbedded && <QueryHeader query={query}/>}
358+
{!props.isEmbedded && <QueryHeader query={query} />}
360359
<div className="row">
361360
<div className="col-12">
362361
{loadingMessage}
363362
<div id="live-plan" className="graph-container">
364363
<div className="float-end">
365-
{state.ended ? "Scroll to zoom." : "Zoom disabled while query is running." } Click stage to view additional statistics
364+
{state.ended ? "Scroll to zoom." : "Zoom disabled while query is running."} Click stage to
365+
view additional statistics
366366
</div>
367367
<svg id="plan-canvas" ref={svgRef} />
368368
</div>

‎presto-ui/src/components/PageTitle.jsx‎

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type Props = {
1919
urls?: string[],
2020
current?: number,
2121
path?: string,
22-
}
22+
};
2323

2424
type State = {
2525
noConnection: boolean,
@@ -28,28 +28,26 @@ type State = {
2828
lastSuccess: number,
2929
modalShown: boolean,
3030
errorText: ?string,
31-
}
31+
};
3232

33-
const ClusterResourceGroupNavBar = ({titles, urls, current = 0} : Props) => {
34-
const classNames = ['navbar-brand inactive', 'navbar-brand'];
35-
const navBarItems = titles.map( (title, index) => {
36-
const classNameIdx = (current === index || !urls?.length) ? 0 : 1;
33+
const ClusterResourceGroupNavBar = ({ titles, urls, current = 0 }: Props) => {
34+
const classNames = ["navbar-brand inactive", "navbar-brand"];
35+
const navBarItems = titles.map((title, index) => {
36+
const classNameIdx = current === index || !urls?.length ? 0 : 1;
3737
return (
3838
<td key={index}>
3939
<span className={classNames[classNameIdx]}>
40-
{ classNameIdx ? <a href={urls?.[index]}>{title}</a> : title}
40+
{classNameIdx ? <a href={urls?.[index]}>{title}</a> : title}
4141
</span>
4242
</td>
4343
);
4444
});
4545

46-
return (
47-
<>{navBarItems}</>
48-
);
46+
return <>{navBarItems}</>;
4947
};
5048

5149
const isOffline = () => {
52-
return window.location.protocol === 'file:';
50+
return window.location.protocol === "file:";
5351
};
5452

5553
export const PageTitle = (props: Props): React.Node => {
@@ -67,47 +65,48 @@ export const PageTitle = (props: Props): React.Node => {
6765
const refreshLoop = () => {
6866
clearTimeout(timeoutId.current);
6967
fetch("/v1/info")
70-
.then(response => response.json())
71-
.then(info => {
72-
setState(prevState => ({
68+
.then((response) => response.json())
69+
.then((info) => {
70+
setState((prevState) => ({
7371
...prevState,
7472
info: info,
7573
noConnection: false,
7674
lastSuccess: Date.now(),
7775
modalShown: false,
7876
}));
7977
//$FlowFixMe$ Bootstrap 5 plugin
80-
$('#no-connection-modal').hide();
78+
$("#no-connection-modal").hide();
8179
timeoutId.current = setTimeout(refreshLoop, 1000);
8280
})
83-
.catch(error => {
84-
setState(prevState => {
81+
.catch((error) => {
82+
setState((prevState) => {
8583
const noConnection = true;
8684
const lightShown = !prevState.lightShown;
8785
const errorText = error;
88-
const shouldShowModal = !prevState.modalShown && (error || (Date.now() - prevState.lastSuccess) > 30 * 1000);
89-
86+
const shouldShowModal =
87+
!prevState.modalShown && (error || Date.now() - prevState.lastSuccess > 30 * 1000);
88+
9089
if (shouldShowModal) {
9190
//$FlowFixMe$ Bootstrap 5 plugin
92-
$('#no-connection-modal').modal('show');
91+
$("#no-connection-modal").modal("show");
9392
}
9493

9594
timeoutId.current = setTimeout(refreshLoop, 1000);
96-
95+
9796
return {
9897
...prevState,
9998
noConnection,
10099
lightShown,
101100
errorText,
102-
modalShown: shouldShowModal || prevState.modalShown
101+
modalShown: shouldShowModal || prevState.modalShown,
103102
};
104103
});
105104
});
106105
};
107106

108107
useEffect(() => {
109108
if (isOffline()) {
110-
setState(prevState => ({
109+
setState((prevState) => ({
111110
...prevState,
112111
noConnection: true,
113112
lightShown: true,
@@ -124,12 +123,12 @@ export const PageTitle = (props: Props): React.Node => {
124123
const renderStatusLight = () => {
125124
if (state.noConnection) {
126125
if (state.lightShown) {
127-
return <span className="status-light status-light-red" id="status-indicator"/>;
126+
return <span className="status-light status-light-red" id="status-indicator" />;
128127
} else {
129-
return <span className="status-light" id="status-indicator"/>;
128+
return <span className="status-light" id="status-indicator" />;
130129
}
131130
} else {
132-
return <span className="status-light status-light-green" id="status-indicator"/>;
131+
return <span className="status-light status-light-green" id="status-indicator" />;
133132
}
134133
};
135134

@@ -147,38 +146,61 @@ export const PageTitle = (props: Props): React.Node => {
147146
<tbody>
148147
<tr>
149148
<td>
150-
<a href="/ui/"><img src={`${props.path || '.'}/assets/logo.png`}/></a>
149+
<a href="/ui/">
150+
<img src={`${props.path || "."}/assets/logo.png`} />
151+
</a>
151152
</td>
152-
<ClusterResourceGroupNavBar titles={props.titles} urls={props.urls} current={props.current} />
153+
<ClusterResourceGroupNavBar
154+
titles={props.titles}
155+
urls={props.urls}
156+
current={props.current}
157+
/>
153158
</tr>
154159
</tbody>
155160
</table>
156161
</div>
157-
<button className="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbar" aria-controls="navbar" aria-expanded="false" aria-label="Toggle navigation">
162+
<button
163+
className="navbar-toggler"
164+
type="button"
165+
data-bs-toggle="collapse"
166+
data-bs-target="#navbar"
167+
aria-controls="navbar"
168+
aria-expanded="false"
169+
aria-label="Toggle navigation"
170+
>
158171
<span className="navbar-toggler-icon"></span>
159172
</button>
160173
<div id="navbar" className="navbar-collapse collapse">
161174
<ul className="nav navbar-nav navbar-right ms-auto">
162175
<li>
163176
<span className="navbar-cluster-info">
164-
<span className="uppercase">Version</span><br/>
165-
<span className="text" id="version-number">{isOffline() ? 'N/A' : info?.nodeVersion?.version}</span>
177+
<span className="uppercase">Version</span>
178+
<br />
179+
<span className="text" id="version-number">
180+
{isOffline() ? "N/A" : info?.nodeVersion?.version}
181+
</span>
166182
</span>
167183
</li>
168184
<li>
169185
<span className="navbar-cluster-info">
170-
<span className="uppercase">Environment</span><br/>
171-
<span className="text" id="environment">{isOffline() ? 'N/A' : info?.environment}</span>
186+
<span className="uppercase">Environment</span>
187+
<br />
188+
<span className="text" id="environment">
189+
{isOffline() ? "N/A" : info?.environment}
190+
</span>
172191
</span>
173192
</li>
174193
<li>
175194
<span className="navbar-cluster-info">
176-
<span className="uppercase">Uptime</span><br/>
195+
<span className="uppercase">Uptime</span>
196+
<br />
177197
<span data-bs-toggle="tooltip" data-bs-placement="bottom" title="Connection status">
178198
{renderStatusLight()}
179199
</span>
180200
&nbsp;
181-
<span className="text" id="uptime">{isOffline() ? 'Offline' : info?.uptime}</span>
201+
<span className="text" id="uptime">
202+
{isOffline() ? "Offline" : info?.uptime}
203+
</span>
182204
</span>
183205
</li>
184206
</ul>

0 commit comments

Comments
 (0)