Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

Commit

Permalink
Open-sourced Correct React Import rule (#58)
Browse files Browse the repository at this point in the history
  • Loading branch information
jukben authored Jul 1, 2019
1 parent fc94d8d commit c1adf0b
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 11 deletions.
47 changes: 36 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,21 @@ AppDispatcher.handleViewAction({

To enforce some rules to our [selectors](/~https://github.com/productboard/pb-frontend/blob/master/docs/flux.md#selectors-1).

1) **Dependency array must be defined as array literal.**
It's better practice to have the list of dependencies inlined rather than in some variable outside of selector definition.
1. **Dependency array must be defined as array literal.**
It's better practice to have the list of dependencies inlined rather than in some variable outside of selector definition.

2) **Dependency array must contain at least one dependency.**
Otherwise it's probably misused selector and developer should use plain (possibly memoized) function.
2. **Dependency array must contain at least one dependency.**
Otherwise it's probably misused selector and developer should use plain (possibly memoized) function.

3) **Function in selector must be defined as arrow literal.**
First for readability we want the function to be inlined and not defined outside of selector definition.
Also, we don't wanna use `function` definition, to avoid possible `this` abuse.
3. **Function in selector must be defined as arrow literal.**
First for readability we want the function to be inlined and not defined outside of selector definition.
Also, we don't wanna use `function` definition, to avoid possible `this` abuse.

4) **Default arguments in selector function are forbidden.**
Unfortunately, JavaScript doesn't play well with default arguments when using memoization on dynamic number of arguments. Therefore we have to disable it to prevent nasty bugs.
4. **Default arguments in selector function are forbidden.**
Unfortunately, JavaScript doesn't play well with default arguments when using memoization on dynamic number of arguments. Therefore we have to disable it to prevent nasty bugs.

5) **All arguments in selector function must be typed.**
Unfortunately if you skip types on arguments, it just uses implicit `any` (probably because of generics used in `select` definition). It's potentially error-prone, so it's good idea to enforce it.
5. **All arguments in selector function must be typed.**
Unfortunately if you skip types on arguments, it just uses implicit `any` (probably because of generics used in `select` definition). It's potentially error-prone, so it's good idea to enforce it.

#### Configuration

Expand Down Expand Up @@ -234,6 +234,31 @@ select(
```

### correct-react-import

Ensure that React is consistently imported without asterisk import.

#### Configuration

```json
{
"rules": {
"correct-react-import": true
}
}
```

#### Example

```text
import * as React from 'react';
~~~~~~~~~~ [Don't import React with asterisk, use `import React from 'react';`]
```

```
import React from 'react';
```

## Install

```
Expand Down
57 changes: 57 additions & 0 deletions src/rules/correctReactImportRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Copyright (c) 2019-present, ProductBoard, Inc.
* All rights reserved.
*/

import * as Lint from "tslint";
import * as ts from "typescript";

type Configuration = {
reference: string;
};

export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING =
"Don't import React with asterisk, use `import React from 'react';`";

public apply(sourceFile: ts.SourceFile): Array<Lint.RuleFailure> {
return this.applyWithWalker(
new CorrectReactImportWalker(sourceFile, this.getOptions())
);
}
}

class CorrectReactImportWalker extends Lint.RuleWalker {
private configuration: Configuration;

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);

const configuration = {
reference: "",
...options.ruleArguments[0]
} as Configuration;

this.configuration = configuration;
}

private getFormattedError(error: string): string {
return (
error +
(this.configuration.reference ? ` ${this.configuration.reference}` : "")
);
}

public visitNamespaceImport(node: ts.NamespaceImport) {
if (
ts.isStringLiteral(node.parent.parent.moduleSpecifier) &&
node.parent.parent.moduleSpecifier.text === "react"
) {
return this.addFailureAtNode(
node,
this.getFormattedError(Rule.FAILURE_STRING)
);
}
super.visitNamespaceImport(node);
}
}
2 changes: 2 additions & 0 deletions test/rules/correct-react-import/test.1.tsx.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import * as React from 'react';
~~~~~~~~~~ [Don't import React with asterisk, use `import React from 'react';`]
1 change: 1 addition & 0 deletions test/rules/correct-react-import/test.2.tsx.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import React from 'react';
5 changes: 5 additions & 0 deletions test/rules/correct-react-import/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"correct-react-import": [true]
}
}
1 change: 1 addition & 0 deletions tslint-pb.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"check-unused-flux-dependencies": true,
"sort-flux-dependencies": true,
"flux-action-dispatch": true,
"correct-react-import": true,
"selectors-format": [
true,
{
Expand Down

0 comments on commit c1adf0b

Please sign in to comment.