Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow jsx-no-bind and jsx-no-lambda to ignore DOMComponents #179

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions src/rules/jsxNoBindRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,19 @@
*/

import * as Lint from "tslint";
import { isCallExpression, isJsxAttribute, isJsxExpression } from "tsutils";
import {
isCallExpression,
isJsxAttribute,
isJsxExpression,
isJsxOpeningElement,
isJsxSelfClosingElement,
} from "tsutils";
import * as ts from "typescript";
import { isDOMComponent } from "../utils";

interface IOption {
allowDOMComponent: boolean;
}

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
Expand All @@ -31,9 +42,16 @@ export class Rule extends Lint.Rules.AbstractRule {
not a semantic one (it doesn't use the type checker). So it may \
have some rare false positives if you define your own .bind function \
and supply this as a parameter.`,
options: null,
optionsDescription: "",
optionExamples: ["true"],
options: {
type: "array",
items: {
type: "string",
enum: ["allow-dom-component"],
},
},
optionsDescription: Lint.Utils.dedent`
Whether to allow function binding with DOM Components`,
optionExamples: [`[true, ["allow-dom-component"]]`],
type: "functionality",
typescriptOnly: false,
};
Expand All @@ -46,16 +64,26 @@ export class Rule extends Lint.Rules.AbstractRule {
// An optional 3rd parameter allows you to pass in a parsed version
// of this.ruleArguments. If used, it is preferred to parse it into
// a more useful object than this.getOptions().
return this.applyWithFunction(sourceFile, walk);
return this.applyWithFunction(sourceFile, walk, {
allowDOMComponent: this.ruleArguments.indexOf("allow-dom-component") !== -1,
});
}
}

function walk(ctx: Lint.WalkContext<void>): void {
function walk(ctx: Lint.WalkContext<IOption>): void {
let currentTagElement: ts.JsxOpeningElement | ts.JsxSelfClosingElement;
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
if (isJsxOpeningElement(node) || isJsxSelfClosingElement(node)) {
currentTagElement = node;
}
if (!isJsxAttribute(node)) {
return ts.forEachChild(node, cb);
}

if (ctx.options.allowDOMComponent && isDOMComponent(currentTagElement)) {
return;
}

const initializer = node.initializer;
if (initializer === undefined || !isJsxExpression(initializer)) {
return;
Expand Down
32 changes: 27 additions & 5 deletions src/rules/jsxNoLambdaRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
import * as Lint from "tslint";
import { isJsxAttribute, isJsxExpression } from "tsutils";
import * as ts from "typescript";
import { isDOMComponent } from "../utils";

interface IOption {
allowDOMComponent: boolean;
}

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
Expand All @@ -29,9 +34,16 @@ export class Rule extends Lint.Rules.AbstractRule {
ES2015 arrow syntax) inside the render call stack works against pure component \
rendering. When doing an equality check between two lambdas, React will always \
consider them unequal values and force the component to re-render more often than necessary.`,
options: null,
optionsDescription: "",
optionExamples: ["true"],
options: {
type: "array",
items: {
type: "string",
enum: ["allow-dom-component"],
},
},
optionsDescription: Lint.Utils.dedent`
Whether to allow anonymous function with DOM Components`,
optionExamples: [`[true, ["allow-dom-component"]]`],
type: "functionality",
typescriptOnly: false,
};
Expand All @@ -40,12 +52,18 @@ export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING = "Lambdas are forbidden in JSX attributes due to their rendering performance impact";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
return this.applyWithFunction(sourceFile, walk, {
allowDOMComponent: this.ruleArguments.indexOf("allow-dom-component") !== -1,
});
}
}

function walk(ctx: Lint.WalkContext<void>) {
function walk(ctx: Lint.WalkContext<IOption>) {
let currentTagElement: ts.JsxOpeningElement | ts.JsxSelfClosingElement;
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
if (ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node)) {
currentTagElement = node;
}
// continue iterations until JsxAttribute will be found
if (isJsxAttribute(node)) {
const { initializer } = node;
Expand All @@ -60,6 +78,10 @@ function walk(ctx: Lint.WalkContext<void>) {
return;
}

if (ctx.options.allowDOMComponent && isDOMComponent(currentTagElement)) {
return;
}

const { expression } = initializer;
if (expression !== undefined && isLambda(expression)) {
return ctx.addFailureAtNode(expression, Rule.FAILURE_STRING);
Expand Down
4 changes: 4 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ export function getDeleteFixForSpaceBetweenTokens(
}
}

export function isDOMComponent(node: ts.JsxOpeningElement | ts.JsxSelfClosingElement): boolean {
return /^[a-z]/.test(node.tagName.getText());
}

function getTotalCharCount(comments: ts.CommentRange[]) {
return comments
.map((comment) => comment.end - comment.pos)
Expand Down
18 changes: 14 additions & 4 deletions test/rules/jsx-no-bind/test.tsx.lint
Original file line number Diff line number Diff line change
@@ -1,18 +1,28 @@
function foo() {
}

export const myButton = (
export const domButton = (
<button onClick={foo.bind(this)}>
~~~~~~~~~~~~~~ [0]
Log something
</button>
);

export const domButton2 = (
<button onClick={foo.bind(this)} />
);

export const myButton = (
<Button onClick={foo.bind(this)}>
~~~~~~~~~~~~~~ [0]
Log something
</Button>
);

export const myButton2 = (
<button onClick={this.foo.bind(this)}>
<Button onClick={this.foo.bind(this)}>
~~~~~~~~~~~~~~~~~~~ [0]
Log something
</button>
</Button>
);

export const selector = (
Expand Down
2 changes: 1 addition & 1 deletion test/rules/jsx-no-bind/tslint.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"rules": {
"jsx-no-bind": true
"jsx-no-bind": [true, "allow-dom-component"]
}
}
22 changes: 17 additions & 5 deletions test/rules/jsx-no-lambda/test.tsx.lint
Original file line number Diff line number Diff line change
@@ -1,27 +1,39 @@
export const myButton = (
<button onClick={() => console.log("clicked")}>
<Button onClick={() => console.log("clicked")}>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
Log something
</button>
</Button>
);

const myButton2 = (
<button onClick={function () {
<Button onClick={function () {
~~~~~~~~~~~~~
// another bad one
~~~~~~~~~~~~~~~~~~~~~~~~~~
}} disabled/>
~~~~~ [0]
)

export const myDOMButton = (
<button onClick={() => console.log("clicked")}>
Log something
</button>
);

const myDOMButton2 = (
<button onClick={function () {
// another bad one
}} disabled/>
)

const handleClick = () => {
// this one is kosher
};

export const myAnchorButton = (
<a href="#" onClick={handleClick}>
<Link href="#" onClick={handleClick}>
Go nowhere, just log something
</a>
</Link>
);

function randomOkFunction() {
Expand Down
2 changes: 1 addition & 1 deletion test/rules/jsx-no-lambda/tslint.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"rules": {
"jsx-no-lambda": true
"jsx-no-lambda": [true, "allow-dom-component"]
}
}