Skip to content

Commit

Permalink
feat (require-listener-teardown): detect inline functions and documen…
Browse files Browse the repository at this point in the history
…t/window
  • Loading branch information
43081j committed Apr 23, 2023
1 parent 842819d commit 22794f7
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 8 deletions.
73 changes: 65 additions & 8 deletions src/rules/require-listener-teardown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,28 @@ const rule: Rule.RuleModule = {
messages: {
noTeardown:
'Event listeners attached in `connectedCallback` should be' +
'torn down during `disconnectedCallback`'
}
'torn down during `disconnectedCallback`',
noArrowBind:
'Using an inline arrow function or `bind(...)` will result in ' +
'creating a new function each time you call add/removeEventListener. ' +
'This will result in the original handler not being removed as ' +
'expected. You should instead store a reference to the function ' +
'and pass that in as your handler.'
},
schema: [
{
type: 'object',
additionalProperties: false,
properties: {
hosts: {
type: 'array',
items: {
type: 'string'
}
}
}
}
]
},

create(context): Rule.RuleListener {
Expand All @@ -33,33 +53,70 @@ const rule: Rule.RuleModule = {
'MethodDefinition[key.name="disconnectedCallback"] ' +
'CallExpression[callee.property.name="removeEventListener"]';
const seen = new Map<string, ESTree.CallExpression>();
const options = context.options[0];
const trackedHosts = options?.hosts ?? ['this', 'window', 'document'];

//----------------------------------------------------------------------
// Helpers
//----------------------------------------------------------------------
const shouldTrackListener = (node: ESTree.MemberExpression): boolean => {
const source = context.getSourceCode();

const text = source.getText(node.object);

return trackedHosts.includes(text);
};
const isInlineFunction = (node: ESTree.Node): boolean =>
node.type === 'ArrowFunctionExpression' ||
node.type === 'FunctionExpression' ||
(node.type === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'bind');

//----------------------------------------------------------------------
// Public
//----------------------------------------------------------------------
const onAddListener = (node: ESTree.CallExpression): void => {
if (
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'ThisExpression'
shouldTrackListener(node.callee)
) {
const arg0 = node.arguments[0];
const source = context.getSourceCode();
const calleeText = source.getText(node.callee.object);
const [arg0, arg1] = node.arguments;

if (isInlineFunction(arg1)) {
context.report({
node: arg1,
messageId: 'noArrowBind'
});
}

if (arg0.type === 'Literal' && typeof arg0.value === 'string') {
seen.set(arg0.value, node);
seen.set(`${calleeText}:${arg0.value}`, node);
}
}
};

const onRemoveListener = (node: ESTree.CallExpression): void => {
if (
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'ThisExpression'
shouldTrackListener(node.callee)
) {
const arg0 = node.arguments[0];
const source = context.getSourceCode();
const calleeText = source.getText(node.callee.object);
const [arg0, arg1] = node.arguments;

if (isInlineFunction(arg1)) {
context.report({
node: arg1,
messageId: 'noArrowBind'
});
}

if (arg0.type === 'Literal' && typeof arg0.value === 'string') {
seen.delete(arg0.value);
seen.delete(`${calleeText}:${arg0.value}`);
}
}
};
Expand Down
75 changes: 75 additions & 0 deletions src/test/rules/require-listener-teardown_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,18 @@ ruleTester.run('require-listener-teardown', rule, {
this.something.addEventListener('x', console.log);
}
}`
},
{
code: `class Foo extends HTMLElement {
connectedCallback() {
this.addEventListener('foo', console.log);
}
}`,
options: [
{
hosts: ['window', 'document']
}
]
}
],

Expand Down Expand Up @@ -159,6 +171,69 @@ ruleTester.run('require-listener-teardown', rule, {
column: 11
}
]
},
{
code: `class Foo extends HMTLElement {
connectedCallback() {
foo.addEventListener('x', console.log);
}
}`,
options: [
{
hosts: ['foo']
}
],
errors: [
{
messageId: 'noTeardown',
line: 3,
column: 11
}
]
},
{
code: `class Foo extends HMTLElement {
connectedCallback() {
this.addEventListener('x', this.foo.bind(this));
}
disconnectedCallback() {
this.removeEventListener('x', this.foo.bind(this));
}
}`,
errors: [
{
messageId: 'noArrowBind',
line: 3,
column: 38
},
{
messageId: 'noArrowBind',
line: 6,
column: 41
}
]
},
{
code: `class Foo extends HMTLElement {
connectedCallback() {
this.addEventListener('x', () => {});
}
disconnectedCallback() {
this.removeEventListener('x', () => {});
}
}`,
errors: [
{
messageId: 'noArrowBind',
line: 3,
column: 38
},
{
messageId: 'noArrowBind',
line: 6,
column: 41
}
]
}
]
});

0 comments on commit 22794f7

Please sign in to comment.