Skip to content

Commit a2243fd

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
no-imperative-dom-api: Handle some UI.Utils and select.add call
Bug: 407751147 Change-Id: I649239978b139284f3b4fc32a8fe77502732e9c4 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6438857 Commit-Queue: Danil Somsikov <[email protected]> Reviewed-by: Philip Pfaffe <[email protected]> Auto-Submit: Danil Somsikov <[email protected]>
1 parent 9c46b51 commit a2243fd

File tree

7 files changed

+194
-5
lines changed

7 files changed

+194
-5
lines changed

front_end/panels/common/common.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
/* eslint-disable rulesdir/no-lit-render-outside-of-view */
5+
/* eslint-disable rulesdir/no-imperative-dom-api */
56

67
import * as Host from '../../core/host/host.js';
78
import * as i18n from '../../core/i18n/i18n.js';

front_end/panels/lighthouse/LighthouseTimespanView.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright 2022 The Chromium Authors. All rights reserved.
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
4+
/* eslint-disable rulesdir/no-imperative-dom-api */
45

56
import * as i18n from '../../core/i18n/i18n.js';
67
import * as Buttons from '../../ui/components/buttons/buttons.js';

scripts/eslint_rules/lib/no-imperative-dom-api.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {domApiDevtoolsExtensions} from './no-imperative-dom-api/dom-api-devtools
1414
import {domApi} from './no-imperative-dom-api/dom-api.ts';
1515
import {DomFragment} from './no-imperative-dom-api/dom-fragment.ts';
1616
import {toolbar} from './no-imperative-dom-api/toolbar.ts';
17+
import {uiUtils} from './no-imperative-dom-api/ui-utils.ts';
1718
import {widget} from './no-imperative-dom-api/widget.ts';
1819
import {createRule} from './tsUtils.ts';
1920
type CallExpression = TSESTree.CallExpression;
@@ -35,6 +36,8 @@ type Subrule = Partial<{
3536
MemberExpression: (node: MemberExpression) => void,
3637
// eslint-disable-next-line @typescript-eslint/naming-convention
3738
NewExpression: (node: NewExpression) => void,
39+
// eslint-disable-next-line @typescript-eslint/naming-convention
40+
CallExpression: (node: CallExpression) => void,
3841
}>;
3942

4043
export default createRule({
@@ -60,6 +63,7 @@ export default createRule({
6063
domApi.create(context),
6164
domApiDevtoolsExtensions.create(context),
6265
toolbar.create(context),
66+
uiUtils.create(context),
6367
widget.create(context),
6468
];
6569

@@ -230,6 +234,13 @@ export const DEFAULT_VIEW = (input, _output, target) => {
230234
}
231235
}
232236
},
237+
CallExpression(node) {
238+
for (const rule of subrules) {
239+
if ('CallExpression' in rule) {
240+
rule.CallExpression?.(node);
241+
}
242+
}
243+
},
233244
'Program:exit'() {
234245
let processedSome = false;
235246
do {

scripts/eslint_rules/lib/no-imperative-dom-api/dom-api.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,12 @@ export const domApi = {
7373
return false;
7474
},
7575
methodCall(
76-
property: Identifier, firstArg: Node, secondArg: Node, domFragment: DomFragment, call: CallExpression) {
76+
property: Identifier, firstArg: Node, secondArg: Node|undefined, domFragment: DomFragment,
77+
call: CallExpression) {
7778
if (isIdentifier(property, 'setAttribute')) {
7879
const attribute = firstArg;
7980
const value = secondArg;
80-
if (attribute.type === 'Literal' && value.type !== 'SpreadElement' && attribute.value) {
81+
if (attribute.type === 'Literal' && value && value.type !== 'SpreadElement' && attribute.value) {
8182
domFragment.attributes.push({key: attribute.value.toString(), value});
8283
return true;
8384
}
@@ -86,6 +87,15 @@ export const domApi = {
8687
domFragment.appendChild(firstArg, sourceCode);
8788
return true;
8889
}
90+
if (domFragment.tagName === 'select' && isIdentifier(property, 'add')) {
91+
if (secondArg) {
92+
const index = domFragment.children.indexOf(DomFragment.getOrCreate(secondArg, sourceCode));
93+
domFragment.insertChildAt(secondArg, index, sourceCode);
94+
} else {
95+
domFragment.appendChild(firstArg, sourceCode);
96+
}
97+
return true;
98+
}
8999
if (isIdentifier(property, 'append')) {
90100
for (const child of call.arguments) {
91101
domFragment.appendChild(child, sourceCode);
@@ -98,7 +108,7 @@ export const domApi = {
98108
}
99109
return true;
100110
}
101-
if (isIdentifier(property, 'insertBefore')) {
111+
if (isIdentifier(property, 'insertBefore') && secondArg) {
102112
const index = domFragment.children.indexOf(DomFragment.getOrCreate(secondArg, sourceCode));
103113
if (index !== -1) {
104114
for (const reference of domFragment.children[index].references) {
@@ -110,7 +120,7 @@ export const domApi = {
110120
return true;
111121
}
112122
}
113-
if (isIdentifier(property, 'insertAdjacentElement')) {
123+
if (isIdentifier(property, 'insertAdjacentElement') && secondArg) {
114124
if (domFragment.parent) {
115125
const index = domFragment.parent.children.indexOf(domFragment);
116126
if (isLiteral(firstArg, 'afterend')) {

scripts/eslint_rules/lib/no-imperative-dom-api/dom-fragment.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ export class DomFragment {
153153
components.push('>');
154154
if (this.textContent) {
155155
components.push(toOutputString(this.textContent));
156-
} else if (this.children?.length) {
156+
}
157+
if (this.children?.length) {
157158
for (const child of this.children || []) {
158159
components.push(...child.toTemplateLiteral(sourceCode, indent + 2));
159160
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
/**
5+
* @fileoverview A library to identify and templatize UI.UIUtils calls.
6+
*/
7+
8+
import type {TSESTree} from '@typescript-eslint/utils';
9+
10+
import {isIdentifier, isMemberExpression} from './ast.ts';
11+
import {DomFragment} from './dom-fragment.ts';
12+
type CallExpression = TSESTree.CallExpression;
13+
type MemberExpression = TSESTree.MemberExpression;
14+
type Identifier = TSESTree.Identifier;
15+
type Node = TSESTree.Node;
16+
17+
export const uiUtils = {
18+
create(context) {
19+
const sourceCode = context.getSourceCode();
20+
return {
21+
CallExpression(node: CallExpression) {
22+
if (!isMemberExpression(
23+
node.callee, n => isMemberExpression(n, n => isIdentifier(n, 'UI'), n => isIdentifier(n, 'UIUtils')),
24+
n => n.type === 'Identifier')) {
25+
return;
26+
}
27+
const functionName = ((node.callee as MemberExpression).property as Identifier).name;
28+
if (functionName === 'createLabel') {
29+
const domFragment = DomFragment.getOrCreate(node, sourceCode);
30+
domFragment.tagName = 'label';
31+
const title = node.arguments[0];
32+
if (title) {
33+
domFragment.textContent = title;
34+
}
35+
const className = node.arguments[1];
36+
if (className) {
37+
domFragment.classList.push(className);
38+
}
39+
const associatedControl = node.arguments[2];
40+
if (associatedControl) {
41+
domFragment.appendChild(associatedControl, sourceCode);
42+
}
43+
}
44+
if (functionName === 'createTextButton') {
45+
const opts = node.arguments[2];
46+
if (opts && opts.type !== 'ObjectExpression') {
47+
return;
48+
}
49+
const domFragment = DomFragment.getOrCreate(node, sourceCode);
50+
domFragment.tagName = 'devtools-button';
51+
const text = node.arguments[0];
52+
if (text) {
53+
domFragment.textContent = text;
54+
}
55+
const clickHandler = node.arguments[1];
56+
if (clickHandler) {
57+
domFragment.eventListeners.push({
58+
key: 'click',
59+
value: clickHandler,
60+
});
61+
}
62+
let variant: string|Node = 'Buttons.Button.Variant.OUTLINED';
63+
if (opts) {
64+
for (const property of opts.properties) {
65+
if (property.type !== 'Property') {
66+
continue;
67+
}
68+
if (isIdentifier(property.key, 'className')) {
69+
domFragment.classList.push(property.value);
70+
}
71+
if (isIdentifier(property.key, 'jslogContext')) {
72+
domFragment.bindings.push({
73+
key: 'jslogContext',
74+
value: property.value,
75+
});
76+
}
77+
if (isIdentifier(property.key, 'variant')) {
78+
variant = property.value;
79+
}
80+
if (isIdentifier(property.key, 'title')) {
81+
domFragment.attributes.push({
82+
key: 'title',
83+
value: property.value,
84+
});
85+
}
86+
if (isIdentifier(property.key, 'icon')) {
87+
domFragment.bindings.push({
88+
key: 'iconName',
89+
value: property.value,
90+
});
91+
}
92+
}
93+
}
94+
domFragment.bindings.push({
95+
key: 'variant',
96+
value: variant,
97+
});
98+
}
99+
if (functionName === 'createOption') {
100+
const domFragment = DomFragment.getOrCreate(node, sourceCode);
101+
domFragment.tagName = 'option';
102+
const title = node.arguments[0];
103+
if (title) {
104+
domFragment.textContent = title;
105+
}
106+
const value = node.arguments[1];
107+
if (value) {
108+
domFragment.attributes.push({
109+
key: 'value',
110+
value,
111+
});
112+
}
113+
const jslogContext = node.arguments[1];
114+
if (jslogContext) {
115+
domFragment.attributes.push({
116+
key: 'jslog',
117+
value: '${VisualLogging.dropDown(' + sourceCode.getText(jslogContext) + ').track({click: true})}',
118+
});
119+
}
120+
}
121+
},
122+
};
123+
}
124+
};

scripts/eslint_rules/tests/no-imperative-dom-api.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,47 @@ export const DEFAULT_VIEW = (input, _output, target) => {
544544
target, {host: input});
545545
};
546546
547+
class SomeWidget extends UI.Widget.Widget {
548+
constructor() {
549+
super();
550+
}
551+
}`,
552+
errors: [{messageId: 'preferTemplateLiterals'}],
553+
},
554+
{
555+
filename: 'front_end/ui/components/component/file.ts',
556+
code: `
557+
class SomeWidget extends UI.Widget.Widget {
558+
constructor() {
559+
super();
560+
const select = document.createElement('select');
561+
select.add(UI.UIUtils.createOption('Option 1', '1', 'option-1'));
562+
this.contentElement.appendChild(UI.UIUtils.createLabel('Some label:', 'some-label', select));
563+
this.contentElement.appendChild(UI.UIUtils.createTextButton('Some button', onClick, {
564+
className: 'some-class',
565+
jslogContext: 'some-button',
566+
variant: Buttons.Button.Variant.PRIMARY,
567+
title: i18nString(UIStrings.someTitle),
568+
iconName: 'some-icon'
569+
}));
570+
}
571+
}`,
572+
output: `
573+
574+
export const DEFAULT_VIEW = (input, _output, target) => {
575+
render(html\`
576+
<div>
577+
<label class="some-label">Some label:
578+
<select>
579+
<option value="1" jslog=\${VisualLogging.dropDown('1').track({click: true})}>Option 1</option>
580+
</select>
581+
</label>
582+
<devtools-button class="some-class" title=\${i18nString(UIStrings.someTitle)} @click=\${onClick}
583+
.jslogContext=\${'some-button'} .variant=\${Buttons.Button.Variant.PRIMARY}>Some button</devtools-button>
584+
</div>\`,
585+
target, {host: input});
586+
};
587+
547588
class SomeWidget extends UI.Widget.Widget {
548589
constructor() {
549590
super();

0 commit comments

Comments
 (0)