Skip to content

Commit 8ef31f7

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
Improve DOM API parsing for preferTemplateLiterals rule
- parse the event listeners - parse the property methods like .classList.add - fix the rendering for empty elements Bug: 400353541 Change-Id: I9d734f156ee45bd10856b3354fc10a11b589339a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6352081 Commit-Queue: Danil Somsikov <[email protected]> Reviewed-by: Philip Pfaffe <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> Auto-Submit: Danil Somsikov <[email protected]>
1 parent e71b7c1 commit 8ef31f7

File tree

2 files changed

+89
-7
lines changed

2 files changed

+89
-7
lines changed

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

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ class DomFragment {
4848
/** @type {string|undefined} */ tagName;
4949
/** @type {Node[]} */ classList = [];
5050
/** @type {{key: string, value: Node}[]} */ attributes = [];
51+
/** @type {{key: string, value: Node}[]} */ style = [];
52+
/** @type {{key: string, value: Node}[]} */ eventListeners = [];
5153
/** @type {Node} */ textContent;
5254
/** @type {DomFragment[]} */ children = [];
5355
/** @type {DomFragment|undefined} */ parent;
@@ -97,13 +99,20 @@ class DomFragment {
9799
for (const attribute of this.attributes || []) {
98100
appendExpression(`${attribute.key}=${attributeValue(toOutputString(attribute.value))}`);
99101
}
102+
for (const eventListener of this.eventListeners || []) {
103+
appendExpression(`@${eventListener.key}=${attributeValue(toOutputString(eventListener.value))}`);
104+
}
105+
if (this.style.length) {
106+
const style = this.style.map(s => `${s.key}:${toOutputString(s.value)}`).join('; ');
107+
appendExpression(`style="${style}"`);
108+
}
100109
if (lineLength > MAX_LINE_LENGTH) {
101110
components.push(`\n${' '.repeat(indent)}`);
102111
}
103112
components.push('>');
104113
if (this.textContent) {
105114
components.push(toOutputString(this.textContent));
106-
} else {
115+
} else if (this.children?.length) {
107116
for (const child of this.children || []) {
108117
components.push(...child.toTemplateLiteral(sourceCode, indent + 2));
109118
}
@@ -172,7 +181,17 @@ module.exports = {
172181
const isMethodCall = isAccessed && grandParent.type === 'CallExpression' && grandParent.callee === parent;
173182
const firstArg = isMethodCall ? /** @type {Node} */(grandParent.arguments[0]) : null;
174183
const secondArg = isMethodCall ? /** @type {Node} */(grandParent.arguments[1]) : null;
175-
184+
const grandGrandParent = grandParent.parent;
185+
const isPropertyMethodCall = isAccessed && grandParent.type === 'MemberExpression' &&
186+
grandParent.object === parent && grandGrandParent.type === 'CallExpression' &&
187+
grandGrandParent.callee === grandParent;
188+
const propertyMethodArgument = isPropertyMethodCall ? /** @type {Node} */ (grandGrandParent.arguments[0]) : null;
189+
const isSubpropertyAssignment = isAccessed && grandParent.type === 'MemberExpression' &&
190+
grandParent.object === parent && grandParent.property.type === 'Identifier' &&
191+
grandGrandParent.type === 'AssignmentExpression' && grandGrandParent.left === grandParent;
192+
const subproperty =
193+
isSubpropertyAssignment && grandParent.property.type === 'Identifier' ? grandParent.property : null;
194+
const subpropertyValue = isSubpropertyAssignment ? /** @type {Node} */ (grandGrandParent.right) : null;
176195
reference.processed = true;
177196
if (isPropertyAssignment && isIdentifier(property, 'className')) {
178197
domFragment.classList.push(propertyValue);
@@ -182,9 +201,23 @@ module.exports = {
182201
const attribute = firstArg;
183202
const value = secondArg;
184203
if (attribute.type === 'Literal' && value.type !== 'SpreadElement') {
185-
domFragment.attributes.push({
186-
key: attribute.value.toString(),
187-
value,
204+
domFragment.attributes.push({key: attribute.value.toString(), value});
205+
}
206+
} else if (isMethodCall && isIdentifier(property, 'addEventListener')) {
207+
const event = firstArg;
208+
const value = secondArg;
209+
if (event.type === 'Literal' && value.type !== 'SpreadElement') {
210+
domFragment.eventListeners.push({key: event.value.toString(), value});
211+
}
212+
} else if (
213+
isPropertyMethodCall && isIdentifier(property, 'classList') && isIdentifier(grandParent.property, 'add')) {
214+
domFragment.classList.push(propertyMethodArgument);
215+
} else if (isSubpropertyAssignment && isIdentifier(property, 'style')) {
216+
const property = subproperty.name.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
217+
if (subpropertyValue.type !== 'SpreadElement') {
218+
domFragment.style.push({
219+
key: property,
220+
value: subpropertyValue,
188221
});
189222
}
190223
} else if (isMethodCall && isIdentifier(property, 'appendChild')) {

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

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ class SomeWidget extends UI.Widget.Widget {
3434
export const DEFAULT_VIEW = (input, _output, target) => {
3535
render(html\`
3636
<div>
37-
<div>
38-
</div>
37+
<div></div>
3938
</div>\`,
4039
target, {host: input});
4140
};
@@ -66,6 +65,56 @@ export const DEFAULT_VIEW = (input, _output, target) => {
6665
target, {host: input});
6766
};
6867
68+
class SomeWidget extends UI.Widget.Widget {
69+
constructor() {
70+
super();
71+
}
72+
}`,
73+
errors: [{messageId: 'preferTemplateLiterals'}],
74+
},
75+
{
76+
filename: 'front_end/ui/components/component/file.ts',
77+
code: `
78+
class SomeWidget extends UI.Widget.Widget {
79+
constructor() {
80+
super();
81+
this.contentElement.classList.add('some-class');
82+
this.contentElement.addEventListener('click', this.onClick.bind(this));
83+
}
84+
}`,
85+
output: `
86+
87+
export const DEFAULT_VIEW = (input, _output, target) => {
88+
render(html\`
89+
<div class="some-class" @click=\${this.onClick.bind(this)}></div>\`,
90+
target, {host: input});
91+
};
92+
93+
class SomeWidget extends UI.Widget.Widget {
94+
constructor() {
95+
super();
96+
}
97+
}`,
98+
errors: [{messageId: 'preferTemplateLiterals'}],
99+
},
100+
{
101+
filename: 'front_end/ui/components/component/file.ts',
102+
code: `
103+
class SomeWidget extends UI.Widget.Widget {
104+
constructor() {
105+
super();
106+
this.contentElement.style.width = '100%';
107+
this.contentElement.style.marginLeft = '10px';
108+
}
109+
}`,
110+
output: `
111+
112+
export const DEFAULT_VIEW = (input, _output, target) => {
113+
render(html\`
114+
<div style="width:100%; margin-left:10px"></div>\`,
115+
target, {host: input});
116+
};
117+
69118
class SomeWidget extends UI.Widget.Widget {
70119
constructor() {
71120
super();

0 commit comments

Comments
 (0)