Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Commit

Permalink
Fixed decorated methods declared after usage (#813) (#852)
Browse files Browse the repository at this point in the history
  • Loading branch information
soon authored and Josh Goldberg committed Mar 31, 2019
1 parent 28e8c34 commit e92a08f
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 5 deletions.
16 changes: 11 additions & 5 deletions src/reactThisBindingIssueRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,20 @@ function walk(ctx: Lint.WalkContext<Options>) {
);
}

function visitClassMember(node: ts.Node) {
if (tsutils.isConstructorDeclaration(node)) {
boundListeners = getSelfBoundListeners(node);
} else if (tsutils.isMethodDeclaration(node)) {
if (isMethodBoundWithDecorators(node, ctx.options.allowedDecorators)) {
boundListeners = boundListeners.add('this.' + node.name.getText());
}
}
}

function cb(node: ts.Node): void {
if (tsutils.isMethodDeclaration(node)) {
// reset variable scope when we encounter a method. Start tracking variables that are instantiated
// in scope so we can make sure new function instances are not passed as JSX attributes
if (isMethodBoundWithDecorators(node, ctx.options.allowedDecorators)) {
boundListeners = boundListeners.add('this.' + node.name.getText());
}
scope = new Scope(undefined);
ts.forEachChild(node, cb);
scope = undefined;
Expand Down Expand Up @@ -289,8 +296,7 @@ function walk(ctx: Lint.WalkContext<Options>) {
declaredMethods.add('this.' + methodName);
}
);
} else if (tsutils.isConstructorDeclaration(node)) {
boundListeners = getSelfBoundListeners(node);
node.members.forEach(visitClassMember);
} else if (tsutils.isJsxElement(node)) {
visitJsxOpeningElement(node.openingElement); // a normal JSX element has-a OpeningElement
} else if (tsutils.isJsxSelfClosingElement(node)) {
Expand Down
5 changes: 5 additions & 0 deletions src/tests/ReactThisBindingIssueRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ describe('reactThisBindingIssueRule', (): void => {
TestHelper.assertNoViolationWithOptions(ruleName, [true, { 'bind-decorators': ['autobind'] }], fileWithDecorator);
});

it('should pass even if a decorated method declared after usage', (): void => {
const file: string = 'test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx';
TestHelper.assertNoViolationWithOptions(ruleName, [true, { 'bind-decorators': ['autobind'] }], file);
});

it('should fail if decorator is not whitelisted in config', () => {
const fileWithDecorator: string = 'test-data/ReactThisBinding/ReactThisBindingIssueWithDecorator-passing.tsx';
const expectedMsg = `A class method is passed as a JSX attribute without having the 'this' reference bound: `;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React = require('react');

declare const autobind: MethodDecorator;

export class MyComponent extends React.Component {
public render() {
return <div onClick={this.listener1} />;
}

@autobind
private listener1(): void {}
}

0 comments on commit e92a08f

Please sign in to comment.