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

Added eslint-plugin-react to the CLIEngine #88

Merged
merged 16 commits into from
Sep 6, 2018
Merged
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
2,189 changes: 1,105 additions & 1,084 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@
"homepage": "https://github.com/Unibeautify/beautifier-eslint#readme",
"peerDependencies": {
"unibeautify": ">= 0.12.1",
"eslint": "^4.19.1 || ^5.0.0"
"eslint": "^4.19.1 || ^5.0.0",
"eslint-plugin-react": "^7.11.1"
},
"devDependencies": {
"@types/eslint": "4.16.3",
"@types/jest": "23.3.1",
"@types/node": "10.9.4",
"@types/read-pkg-up": "3.0.1",
"eslint": "5.5.0",
"eslint": "^5.4.0",
"eslint-plugin-react": "^7.11.1",
"jest": "23.5.0",
"rimraf": "2.6.2",
"ts-jest": "23.1.4",
Expand Down
14 changes: 14 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,16 @@ export const beautifier: Beautifier = {
name: "ESLint",
package: "eslint",
},
{
type: DependencyType.Node,
name: "ESLint-plugin-React",
package: "eslint-plugin-react",
optional: true,
},
],
options: {
JavaScript: options.JavaScript,
JSX: options.JavaScript,
},
resolveConfig: ({ filePath, dependencies }) => {
if (!filePath) {
Expand All @@ -51,6 +58,9 @@ export const beautifier: Beautifier = {
}: BeautifierBeautifyData) {
return new Promise<string>((resolve, reject) => {
const { CLIEngine } = dependencies.get<NodeDependency>("ESLint").package;
const reactPlugin = dependencies.get<NodeDependency>(
"ESLint-plugin-React"
);
const config = (beautifierConfig && beautifierConfig.config) || {};
const { rules, parserOptions, env }: Config = config;
const parseOpts: ParserOptions =
Expand All @@ -67,6 +77,10 @@ export const beautifier: Beautifier = {
rules: finalOptions,
};
const cli: CLIEngine = new CLIEngine(cliOptions);
// tslint:disable-next-line:promise-must-complete
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a bug with https://github.com/Microsoft/tslint-microsoft-contrib/
If you can create a much smaller test case, you could report an issue 👍 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like any if is a branch, and doesn't bother looking what it actually does to consider whether the promise completes or not (since we are using return new Promise<string>((resolve, reject) => {).

Maybe it's safer to remove that block and change the returns at the bottom to return Promise.resolve.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not make any major changes yet. We can try different approaches in a later PR 👍 .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Glavin001 would you mind creating a small test case and make an issue there? I think you can way better explain that error case.

if (reactPlugin.isInstalled) {
cli.addPlugin("eslint-plugin-react", reactPlugin.package);
}
const report: LintReport = cli.executeOnText(text);
const result: LintResult = report.results[0];
if (result.output) {
Expand Down
30 changes: 15 additions & 15 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "capitalized-comments": [
// [""],
Expand All @@ -169,7 +169,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
"comma-style": [
["comma_first"],
Expand All @@ -182,7 +182,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "computed-property-spacing": [
// [""],
Expand All @@ -200,7 +200,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "func-call-spacing": [
// [""],
Expand Down Expand Up @@ -235,7 +235,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "jsx-quotes": [
// [""],
Expand All @@ -257,7 +257,7 @@ const options: BeautifierOptions = {
} else {
return 0;
}
}
},
],
// "linebreak-style": [
// [""],
Expand Down Expand Up @@ -291,7 +291,7 @@ const options: BeautifierOptions = {
return [2, {"ignoreChainWithDepth": 2 }];
}
return 0;
}
},
],
// "no-lonely-if": [
// [""],
Expand All @@ -312,7 +312,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "no-unneeded-ternary": [
// [""],
Expand Down Expand Up @@ -345,7 +345,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "object-property-newline": [
// [""],
Expand Down Expand Up @@ -390,7 +390,7 @@ const options: BeautifierOptions = {
} else {
return [2, options.quotes];
}
}
},
],
"semi": [
["end_with_semicolon"],
Expand All @@ -403,7 +403,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "semi-spacing": [
// [""],
Expand Down Expand Up @@ -436,7 +436,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
"space-in-parens": [
["space_in_paren"],
Expand All @@ -449,7 +449,7 @@ const options: BeautifierOptions = {
default:
return 0;
}
}
},
],
// "space-infix-ops": [
// [""],
Expand Down Expand Up @@ -498,7 +498,7 @@ const options: BeautifierOptions = {
return [2, options.arrow_parens];
}
return 0;
}
},
],
// "arrow-spacing": [
// [""],
Expand Down Expand Up @@ -580,7 +580,7 @@ const options: BeautifierOptions = {
// (options): any => {
// }
// ]
}
},
};
// unibeautify:enable
export default options;
29 changes: 29 additions & 0 deletions test/preferBeautifierConfig/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"parserOptions": {
"ecmaFeatures": {
"jsx": true
}
},
"globals": {
"createReactClass": true
},
"plugins": ["react"],
"extends": ["plugin:react/recommended"],
"rules": {
"react/jsx-wrap-multilines": [
"error", {
"declaration": "parens-new-line",
"assignment": "parens-new-line",
"return": "parens-new-line",
"arrow": "parens-new-line",
"condition": "parens-new-line",
"logical": "parens-new-line",
"prop": "parens-new-line"
}
],
"indent": [
"error", 4
],
"brace-style": ["error", "1tbs"]
}
}
7 changes: 0 additions & 7 deletions test/preferBeautifierConfig/.eslintrc.yml

This file was deleted.

12 changes: 6 additions & 6 deletions test/preferBeautifierConfig/indentConfig.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ test(`should successfully beautify JavaScript text with indent_size=4 with space
options: {
JavaScript: {
ESLint: {
prefer_beautifier_config: true
}
prefer_beautifier_config: true,
},
} as any,
},
text,
Expand All @@ -36,8 +36,8 @@ test(`should not beautify because we are not passing in file path`, () => {
options: {
JavaScript: {
ESLint: {
prefer_beautifier_config: true
}
prefer_beautifier_config: true,
},
} as any,
},
text,
Expand All @@ -58,8 +58,8 @@ test(`should not beautify because file path does not find a config`, () => {
options: {
JavaScript: {
ESLint: {
prefer_beautifier_config: true
}
prefer_beautifier_config: true,
},
} as any,
},
text,
Expand Down
44 changes: 44 additions & 0 deletions test/preferBeautifierConfig/pluginReact.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { newUnibeautify, Beautifier } from "unibeautify";
import beautifier from "../../src";
import * as path from "path";
const filePath: string = path.resolve(__dirname, "test.js");
test(`should successfully beautify JavaScript React with jsx-wrap-multilines`, () => {
const unibeautify = newUnibeautify();
unibeautify.loadBeautifier(beautifier);
const text = `
var Test = createReactClass({
render: function () {
return (<div>
<p>Hello</p>
</div>);
}
});
`;
const beautifierResult = `
var Test = createReactClass({
render: function () {
return (
<div>
<p>Hello</p>
</div>
);
}
});
`;
return unibeautify
.beautify({
filePath,
languageName: "JSX",
options: {
JSX: {
ESLint: {
prefer_beautifier_config: true,
},
} as any,
},
text,
})
.then(results => {
expect(results).toBe(beautifierResult);
});
});
Loading