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

Use ESLint to run TSlint #3887

Merged
merged 16 commits into from Jan 9, 2020
3 changes: 2 additions & 1 deletion .circleci/config.yml
Expand Up @@ -76,7 +76,8 @@ jobs:
- checkout
- restore_cache: *restore-node-modules-cache
- attach_workspace: { at: '.' }
- run: mkdir -p ./reports/tslint ./reports/stylelint
- run: mkdir -p ./reports/eslint ./reports/stylelint
# we need to compile the lint rules for blueprint
- run: yarn compile --scope "@blueprintjs/tslint-config"
- run: yarn lint
- store_test_results: { path: ./reports }
Expand Down
5 changes: 5 additions & 0 deletions .eslintrc.json
@@ -0,0 +1,5 @@
{
"extends": [
"./packages/eslint-config"
]
}
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Expand Up @@ -39,7 +39,7 @@ A typical contributor workflow looks like this:
- Linting is best handled by your editor for real-time feedback (see
[Editor integration](https://github.com/palantir/blueprint/wiki/Editor-integration)). Run
`yarn lint` to be 100% safe.
- TypeScript lint errors can often be automatically fixed by TSLint. Run lint fixes with `yarn lint-fix`.
- TypeScript lint errors can often be automatically fixed by ESLint. Run lint fixes with `yarn lint-fix`.
Copy link
Contributor

Choose a reason for hiding this comment

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

yarn lint-fix doesn't match what's in the next file. I would revert the script name change and keep it as lint-fix

1. Submit a Pull Request on GitHub and fill out the template.
- ⚠️ __DO NOT enable CircleCI for your fork of Blueprint.__ Our build
will run on your fork when you open a PR. You can run NPM scripts locally
Expand Down
6 changes: 3 additions & 3 deletions packages/core/package.json
Expand Up @@ -31,10 +31,10 @@
"dist:css": "css-dist lib/css/*.css",
"dist:variables": "generate-css-variables common/_colors.scss common/_color-aliases.scss common/_variables.scss",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this syntax was intentional since we don't intend to run this command with npm-run-all "lint:*"

"lint:es": "es-lint",
"lint:fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso test:karma",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down
9 changes: 7 additions & 2 deletions packages/core/src/components/html/html.md
Expand Up @@ -40,9 +40,14 @@ See the [Running text](#core/typography.running-text) documentation for more inf

@## Linting

The [**@blueprintjs/eslint-config**](https://www.npmjs.com/package/@blueprintjs/eslint-config)
NPM package provides advanced configuration for [ESLint](https://eslint.org/). Blueprint is
currently transitioning from [TSLint](https://palantir.github.io/tslint/) to ESLint, and as
such, rules are being migrated from TSLint to ESLint. In the meantime, some TSLint rules are
being run using ESLint.

The [**@blueprintjs/tslint-config**](https://www.npmjs.com/package/@blueprintjs/tslint-config)
NPM package provides advanced configuration for [TSLint](http://palantir.github.io/tslint/),
including a custom `blueprint-html-components` rule that will warn on usages of
NPM package includes a custom `blueprint-html-components` rule that will warn on usages of
JSX intrinsic elements (`<h1>`) that have a Blueprint alternative (`<H1>`). See
the package's [README](https://www.npmjs.com/package/@blueprintjs/tslint-config)
for usage instructions.
12 changes: 11 additions & 1 deletion packages/core/src/docs/classes.md
Expand Up @@ -96,4 +96,14 @@ requires several things:

@## Linting

The [**@blueprintjs/tslint-config**](https://www.npmjs.com/package/@blueprintjs/tslint-config) NPM package provides advanced configuration for [TSLint](http://palantir.github.io/tslint/), including a custom `blueprint-classes-constants` rule that will detect and warn about hardcoded `pt-`prefixed strings. See the package's [README](https://www.npmjs.com/package/@blueprintjs/tslint-config) for usage instructions.
The [**@blueprintjs/eslint-config**](https://www.npmjs.com/package/@blueprintjs/eslint-config)
NPM package provides advanced configuration for [ESLint](https://eslint.org/). Blueprint is
currently transitioning from [TSLint](https://palantir.github.io/tslint/) to ESLint, and as
such, rules are being migrated from TSLint to ESLint. In the meantime, some TSLint rules are
being run using ESLint.

The [**@blueprintjs/tslint-config**](https://www.npmjs.com/package/@blueprintjs/tslint-config)
NPM package includes a custom `blueprint-html-components` rule that will warn on usages of
JSX intrinsic elements (`<h1>`) that have a Blueprint alternative (`<H1>`). See
the package's [README](https://www.npmjs.com/package/@blueprintjs/tslint-config)
for usage instructions.
6 changes: 3 additions & 3 deletions packages/datetime/package.json
Expand Up @@ -23,10 +23,10 @@
"dist:bundle": "cross-env NODE_ENV=production webpack",
"dist:css": "css-dist lib/css/*.css",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso test:karma",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down
6 changes: 3 additions & 3 deletions packages/docs-app/package.json
Expand Up @@ -8,10 +8,10 @@
"clean": "rm -rf dist/*",
"dev": "webpack-dev-server --config ./webpack.config.js --host 0.0.0.0",
"dist": "cross-env NODE_ENV=production yarn bundle",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "exit 0",
"verify": "run-p dist lint"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/docs-theme/package.json
Expand Up @@ -23,10 +23,10 @@
"dist:bundle": "cross-env NODE_ENV=production webpack",
"dist:css": "css-dist lib/css/*.css",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"verify": "npm-run-all compile -p dist lint"
},
"dependencies": {
Expand Down
41 changes: 41 additions & 0 deletions packages/eslint-config/index.js
@@ -0,0 +1,41 @@
/*
* Copyright 2019 Palantir Technologies, Inc. All rights reserved.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

module.exports = {
env: {
browser: true,
mocha: true,
node: true,
},
plugins: [
"@typescript-eslint",
"@typescript-eslint/tslint",
],
parser: "@typescript-eslint/parser",
parserOptions: {
sourceType: "module",
ecmaFeatures: {
jsx: true,

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra newline

},
project: "**/tsconfig.json",
},
rules: {
// run the tslint rules which are not yet converted (run inside eslint)
"@typescript-eslint/tslint/config": ["error", {
"lintFile": "../../tslint.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: inconsistent indentation

}]
}
}
18 changes: 18 additions & 0 deletions packages/eslint-config/package.json
@@ -0,0 +1,18 @@
{
"name": "@blueprintjs/eslint-config",
"version": "0.1.0",
"description": "ESLint configuration for @blueprintjs packages",
"dependencies": {
"@typescript-eslint/eslint-plugin": "^2.15.0",
"@typescript-eslint/eslint-plugin-tslint": "^2.15.0",
"@typescript-eslint/parser": "^2.15.0",
"eslint": "^6.7.2"
},
"repository": {
"type": "git",
"url": "git@github.com:palantir/blueprint.git",
"directory": "packages/eslint-config"
},
"author": "Palantir Technologies",
"license": "Apache-2.0"
}
6 changes: 3 additions & 3 deletions packages/icons/package.json
Expand Up @@ -25,10 +25,10 @@
"dist:css": "css-dist lib/css/*.css",
"dist:variables": "generate-css-variables generated/_icon-variables.scss",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down
6 changes: 3 additions & 3 deletions packages/landing-app/package.json
Expand Up @@ -8,10 +8,10 @@
"clean": "rm -rf dist/*",
"dev": "webpack-dev-server --config ./webpack.config.js",
"dist": "cross-env NODE_ENV=production yarn bundle",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "exit 0",
"verify": "run-p dist lint"
},
Expand Down
48 changes: 48 additions & 0 deletions packages/node-build-scripts/es-lint.js
@@ -0,0 +1,48 @@
#!/usr/bin/env node

/**
* @license Copyright 2017 Palantir Technologies, Inc. All rights reserved.
* @fileoverview Runs ESLint, with support for generating JUnit report
*/

// @ts-check
"use strict";

const path = require("path");
const fs = require("fs");
const { junitReportPath } = require("./utils");
const { spawn, execSync } = require('child_process');
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: quotemark style

const glob = require("glob");

let format = "codeframe";
let out;
let outputStream = process.stdout;
if (process.env.JUNIT_REPORT_PATH != null) {
format = "junit";
out = junitReportPath("eslint");
console.info(`TSLint report will appear in ${out}`);
outputStream = fs.createWriteStream(out, { flags: "w+" });
}

const gitRoot = execSync("git rev-parse --show-toplevel").toString().trim();
const commandLineOptions = ["-c", path.join(gitRoot, "./.eslintrc.json"), "--color", "-f", format];
if (process.argv.includes("--fix")) {
commandLineOptions.push("--fix")
}

const fileGlob = "{src, test}/**/*.tsx";
const absoluteFileGlob = path.resolve(process.cwd(), fileGlob);
const anyFilesToLint = glob.sync(absoluteFileGlob)
if (anyFilesToLint.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why we need to add this... does eslint complain if there are no files?

console.log(`Not running ESLint because no files match the glob "${fileGlob}"`)
process.exit();
}

const eslint = spawn("eslint", [...commandLineOptions, absoluteFileGlob]);

eslint.stdout.pipe(outputStream);
eslint.stderr.pipe(process.stderr);

eslint.on('close', code => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: quotemark style

process.exitCode = code;
})
3 changes: 2 additions & 1 deletion packages/node-build-scripts/package.json
Expand Up @@ -12,10 +12,11 @@
"generate-icons-source": "./generate-icons-source.js",
"sass-compile": "./sass-compile.sh",
"sass-lint": "./sass-lint.js",
"ts-lint": "./ts-lint.js"
"es-lint": "./es-lint.js"
},
"dependencies": {
"autoprefixer": "^9.6.1",
"eslint": "^6.7.2",
"node-sass": "^4.12.0",
"node-sass-chokidar": "^1.3.5",
"node-sass-package-importer": "^5.3.2",
Expand Down
49 changes: 0 additions & 49 deletions packages/node-build-scripts/ts-lint.js

This file was deleted.

6 changes: 3 additions & 3 deletions packages/select/package.json
Expand Up @@ -23,10 +23,10 @@
"dist:bundle": "cross-env NODE_ENV=production webpack",
"dist:css": "css-dist lib/css/*.css",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso test:karma",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down
6 changes: 3 additions & 3 deletions packages/table-dev-app/package.json
Expand Up @@ -8,10 +8,10 @@
"clean": "rm -rf dist/*",
"dev": "webpack-dev-server --config ./webpack.config.js",
"dist": "cross-env NODE_ENV=production yarn bundle",
"lint": "npm-run-all -p lint:scss lint:ts",
"lint": "npm-run-all -p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "exit 0",
"verify": "npm-run-all -p dist lint"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/table/package.json
Expand Up @@ -23,10 +23,10 @@
"dist:bundle": "cross-env NODE_ENV=production webpack",
"dist:css": "css-dist lib/css/*.css",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso test:karma",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/test-commons/package.json
Expand Up @@ -8,7 +8,7 @@
"compile": "tsc -p ./src",
"clean": "rm -rf lib/*",
"dev": "yarn compile --watch",
"lint": "ts-lint",
"lint": "es-lint",
"test": "exit 0"
},
"dependencies": {
Expand Down
6 changes: 3 additions & 3 deletions packages/timezone/package.json
Expand Up @@ -23,10 +23,10 @@
"dist:bundle": "cross-env NODE_ENV=production webpack",
"dist:css": "css-dist lib/css/*.css",
"dist:verify": "assert-package-layout",
"lint": "run-p lint:scss lint:ts",
"lint": "run-p lint:scss lint:es",
"lint:scss": "sass-lint",
"lint:ts": "ts-lint",
"lint-fix": "ts-lint --fix",
"lint:es": "es-lint",
"lint-fix": "es-lint --fix",
"test": "run-s test:typeCheck test:iso test:karma",
"test:typeCheck": "tsc -p ./test",
"test:iso": "mocha test/isotest.js",
Expand Down