Skip to content

Commit

Permalink
Retire TSlint (#1950)
Browse files Browse the repository at this point in the history
This PR retires TSLint in favour of ESLint. Consolidating the linting rules meant
I needed to make a call about what the rules should be going forward - I'm very
happy to discuss / change them. There's a lot of tedious quote changes, but I
think that was unavoidable given the disparate linting configs.

I also expanded the linting / type-checking to cover src-framer. I did my best 
to patch up the code here.
  • Loading branch information
pugnascotia authored May 31, 2019
1 parent 9182922 commit 190e6b4
Show file tree
Hide file tree
Showing 187 changed files with 1,486 additions and 1,178 deletions.
9 changes: 9 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
dist
node_modules
es
lib
types
eui.d.ts
**/*.snap.js
**/assets/**/*.js
src-docs/src/views/icon/icon_example.js
src-framer/code/canvas.tsx
30 changes: 28 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
module.exports = {
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaFeatures: {
jsx: true
}
},
settings: {
"import/resolver": {
node: {
extensions: [".js"]
extensions: [".ts", ".tsx", ".js", ".json"]
},
webpack: {
config: "./src-docs/webpack.config.js"
}
},
react: {
version: "detect"
}
},
extends: [
"@elastic/eslint-config-kibana",
"plugin:@typescript-eslint/recommended",
// Prettier options need to come last, in order to override other style
// rules.
"prettier/react",
Expand All @@ -23,7 +33,23 @@ module.exports = {
],
rules: {
"prefer-template": "error",
"local/i18n": "error"
"local/i18n": "error",
"no-use-before-define": "off",
"quotes": ["warn", "single", "avoid-escape"],

"@typescript-eslint/array-type": ["error", "array-simple"],
"@typescript-eslint/camelcase": "off",
"@typescript-eslint/class-name-casing": "off",
"@typescript-eslint/explicit-function-return-type": "off",
"@typescript-eslint/explicit-member-accessibility": "off",
"@typescript-eslint/indent": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-parameter-properties": "off",
"@typescript-eslint/no-triple-slash-reference": "off",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_", "ignoreRestSiblings": true }],
"@typescript-eslint/no-use-before-define": "off",
},
env: {
jest: true
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Added ability to update `EuiInMemoryTable` `sorting` prop and remove columns after sorting is applied ([#1972](https://github.com/elastic/eui/pull/1972))
- Added `onToggle` callback to `EuiAccordion` ([#1974](https://github.com/elastic/eui/pull/1974))
- Removed `options` `defaultProps` value from `EuiSuperSelect` ([#1975](https://github.com/elastic/eui/pull/1975))
- Remove TSlint and perform all linting through ESLint ([#1950](https://github.com/elastic/eui/pull/1950))

**Bug fixes**

Expand Down
39 changes: 17 additions & 22 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,12 @@
"build": "yarn extract-i18n-strings && node ./scripts/compile-clean.js && node ./scripts/compile-eui.js && node ./scripts/compile-scss.js $npm_package_name",
"compile-icons": "node ./scripts/compile-icons.js && prettier --write --loglevel=warn \"./src/components/icon/assets/**/*.js\"",
"extract-i18n-strings": "node ./scripts/babel/fetch-i18n-strings",
"lint": "yarn lint-es && yarn lint-ts && yarn lint-sass && yarn lint-framer",
"lint-fix": "yarn lint-es-fix && yarn lint-ts-fix",
"lint-es": "eslint --cache --ignore-pattern \"**/*.snap.js\", --ignore-pattern \"**/assets/**/*.js\" \"src/**/*.js\" \"src-docs/**/*.js\"",
"lint-es-fix": "eslint --fix --cache --ignore-pattern \"**/*.snap.js\", \"**/assets/**/*.js\" \"src/**/*.js\" \"src-docs/**/*.js\"",
"lint": "yarn tsc --noEmit && yarn lint-es && yarn lint-sass",
"lint-fix": "yarn lint-es-fix",
"lint-es": "eslint --cache '{src,src-docs,src-framer}/**/*.{ts,tsx,js}'",
"lint-es-fix": "yarn lint-es --fix",
"lint-sass": "sass-lint -v --max-warnings 0",
"lint-sass-fix": "sass-lint-auto-fix -c ./.sass-lint-fix.yml",
"lint-ts": "tsc -p ./tsconfig.json --noEmit && tslint -c ./tslint.yaml -p ./tsconfig.json",
"lint-ts-fix": "tslint -c ./tslint.yaml -p ./tsconfig.json --fix",
"lint-framer": "tslint -c ./tslint.yaml -p ./src-framer/tsconfig.json",
"lint-framer-fix": "tslint -c ./tslint.yaml -p ./src-framer/tsconfig.json --fix",
"test": "yarn lint && yarn test-unit",
"test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.json",
"start-test-server": "webpack-dev-server --config src-docs/webpack.config.js --port 9999",
Expand Down Expand Up @@ -90,9 +86,11 @@
"@types/react-is": "~16.3.0",
"@types/react-virtualized": "^9.18.6",
"@types/uuid": "^3.4.4",
"@typescript-eslint/eslint-plugin": "^1.9.0",
"@typescript-eslint/parser": "^1.9.0",
"autoprefixer": "^7.1.5",
"babel-core": "7.0.0-bridge.0",
"babel-eslint": "^8.0.1",
"babel-eslint": "^10.0.1",
"babel-jest": "^24.1.0",
"babel-loader": "^8.0.4",
"babel-plugin-add-module-exports": "^1.0.0",
Expand All @@ -115,18 +113,18 @@
"enzyme": "^3.9.0",
"enzyme-adapter-react-16": "^1.9.1",
"enzyme-to-json": "^3.3.0",
"eslint": "^4.9.0",
"eslint": "^5.16.0",
"eslint-config-prettier": "^4.2.0",
"eslint-import-resolver-webpack": "^0.8.3",
"eslint-plugin-babel": "^4.1.2",
"eslint-plugin-import": "^2.8.0",
"eslint-plugin-jest": "^21.6.2",
"eslint-plugin-jsx-a11y": "^6.0.2",
"eslint-import-resolver-webpack": "^0.11.1",
"eslint-plugin-babel": "^5.3.0",
"eslint-plugin-import": "^2.17.2",
"eslint-plugin-jest": "^22.5.1",
"eslint-plugin-jsx-a11y": "^6.2.1",
"eslint-plugin-local": "^1.0.0",
"eslint-plugin-mocha": "^4.11.0",
"eslint-plugin-mocha": "^5.3.0",
"eslint-plugin-prefer-object-spread": "^1.2.1",
"eslint-plugin-prettier": "^3.0.1",
"eslint-plugin-react": "^7.4.0",
"eslint-plugin-prettier": "^3.1.0",
"eslint-plugin-react": "^7.13.0",
"file-loader": "^1.1.11",
"findup": "^0.1.5",
"fork-ts-checker-webpack-plugin": "^0.4.4",
Expand Down Expand Up @@ -170,9 +168,6 @@
"sinon": "^4.4.8",
"start-server-and-test": "^1.1.4",
"style-loader": "^0.19.0",
"tslint": "^5.11.0",
"tslint-config-prettier": "^1.18.0",
"tslint-plugin-prettier": "^2.0.1",
"typescript": "^3.3.3",
"uglifyjs-webpack-plugin": "^2.0.1",
"url-loader": "^1.0.1",
Expand All @@ -197,6 +192,6 @@
"prop-types": "^15.5.0",
"react": "^16.8",
"react-dom": "^16.8",
"typescript": "^3.3.0"
"typescript": "^3.3.3"
}
}
44 changes: 26 additions & 18 deletions scripts/dtsgenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const baseDir = path.resolve(__dirname, '..');
const srcDir = path.resolve(baseDir, 'src');

function hasParentIndex(pathToFile) {
const isIndexFile = path.basename(pathToFile, path.extname(pathToFile)) === 'index';
const isIndexFile =
path.basename(pathToFile, path.extname(pathToFile)) === 'index';
try {
const fileDirectory = path.dirname(pathToFile);
const parentIndex = findup.sync(
Expand All @@ -27,9 +28,16 @@ const generator = dtsGenerator({
name: '@elastic/eui',
project: baseDir,
out: 'eui.d.ts',
exclude: ['node_modules/**/*.d.ts', 'src/custom_typings/**/*.d.ts'],
exclude: [
'node_modules/**/*.d.ts',
'*/custom_typings/**/*.d.ts',
'src-framer/**/*',
],
resolveModuleId(params) {
if (path.basename(params.currentModuleId) === 'index' && !hasParentIndex(path.resolve(baseDir, params.currentModuleId))) {
if (
path.basename(params.currentModuleId) === 'index' &&
!hasParentIndex(path.resolve(baseDir, params.currentModuleId))
) {
// this module is exporting from an `index(.d)?.ts` file, declare its exports straight to @elastic/eui module
return '@elastic/eui';
} else {
Expand All @@ -43,20 +51,20 @@ const generator = dtsGenerator({
},
resolveModuleImport(params) {
// only intercept relative imports (don't modify node-modules references)
const importFromBaseDir = path.resolve(baseDir, path.dirname(params.currentModuleId));
const importFromBaseDir = path.resolve(
baseDir,
path.dirname(params.currentModuleId)
);
const isFromEuiSrc = importFromBaseDir.startsWith(srcDir);
const isRelativeImport = isFromEuiSrc && params.importedModuleId[0] === '.';

if (isRelativeImport) {
// if importing from an `index` file (directly or targeting a directory with an index),
// then if there is no parent index file this should import from @elastic/eui
const importPathTarget = resolve.sync(
params.importedModuleId,
{
basedir: importFromBaseDir,
extensions: ['.ts', '.tsx', '.d.ts'],
}
);
const importPathTarget = resolve.sync(params.importedModuleId, {
basedir: importFromBaseDir,
extensions: ['.ts', '.tsx', '.d.ts'],
});

const isIndexFile = importPathTarget.endsWith('/index.ts');
const isModuleIndex = isIndexFile && !hasParentIndex(importPathTarget);
Expand Down Expand Up @@ -87,28 +95,28 @@ generator.then(() => {

fs.writeFileSync(
defsFilePath,
fs.readFileSync(defsFilePath).toString()
fs
.readFileSync(defsFilePath)
.toString()
.replace(/\/\/\/\W+<reference.*/g, '') // 1.
.replace(/import\("src\/(.*?)"\)/g, 'import("@elastic/eui/src/$1")') // 2.
.replace( // start 3.
.replace(
// start 3.
// find any singular `declare module { ... }` block
// {.*?^} matches anything until a } starts a new line (via `m` regex option, and `s` is dotall)
//
// aren't regex really bad for this? Yes.
// However, @babel/preset-typescript doesn't understand some syntax generated in eui.d.ts
// and the tooling around typescript's parsing & code generation is lacking and undocumented
// so... because this works with the guarantee that the newline-brace combination matches a module...
/declare module '(.*?)' {.*?^}/smg,
/declare module '(.*?)' {.*?^}/gms,
(module, moduleName) => {
// `moduleName` is the namespace for this ambient module
return module.replace(
// replace relative imports by attaching them to the module's namespace
/import\("([.]{1,2}\/.*?)"\)/g,
(importStatement, importPath) => {
const target = path.join(
path.dirname(moduleName),
importPath
);
const target = path.join(path.dirname(moduleName), importPath);
return `import ("${target}")`;
}
);
Expand Down
6 changes: 4 additions & 2 deletions scripts/eslint-plugin-i18n/i18n.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Enforce EuiI18n token names & variable names in render prop

/* eslint-disable @typescript-eslint/no-var-requires */

const path = require('path');

function attributesArrayToLookup(attributesArray) {
Expand All @@ -13,7 +15,7 @@ function attributesArrayToLookup(attributesArray) {
}

function getDefinedValues(valuesNode) {
if (valuesNode == null) return new Set();
if (valuesNode == null || valuesNode.expression.properties == null) return new Set();
return valuesNode.expression.properties.reduce(
(valueNames, property) => {
valueNames.add(property.key.name);
Expand Down Expand Up @@ -239,7 +241,7 @@ module.exports = {
// default is a function
// validate the destructured param defined by default function match the values
const defaultFn = attributes.default.expression;
const objProperties = defaultFn.params ? defaultFn.params[0].properties : [];
const objProperties = defaultFn.params && defaultFn.params[0] ? defaultFn.params[0].properties : [];
const expectedNames = new Set(objProperties.map(property => property.key.name));
if (areSetsEqual(valueNames, expectedNames) === false) {
context.report({
Expand Down
6 changes: 6 additions & 0 deletions src-docs/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
extends: '../.eslintrc.js',
rules: {
'@typescript-eslint/no-var-requires': 'off',
}
}
8 changes: 4 additions & 4 deletions src-docs/src/components/guide_section/guide_section.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,16 +367,16 @@ export class GuideSection extends Component {
sourceObject => sourceObject.type === name
);
const npmImports = code
.replace(/(from )'(..\/)+src\/components(\/?';)/, `from '@elastic/eui';`)
.replace(/(from )'(..\/)+src\/components(\/?';)/, "from '@elastic/eui';")
.replace(
/(from )'(..\/)+src\/services(\/?';)/,
`from '@elastic/eui/lib/services';`
"from '@elastic/eui/lib/services';"
)
.replace(
/(from )'(..\/)+src\/experimental(\/?';)/,
`from '@elastic/eui/lib/experimental';`
"from '@elastic/eui/lib/experimental';"
)
.replace(/(from )'(..\/)+src\/components\/.*?';/, `from '@elastic/eui';`);
.replace(/(from )'(..\/)+src\/components\/.*?';/, "from '@elastic/eui';");

return (
<div key={name} ref={name}>
Expand Down
20 changes: 10 additions & 10 deletions src-docs/src/i18ntokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,11 @@
"highlighting": "string",
"loc": {
"start": {
"line": 75,
"line": 77,
"column": 12
},
"end": {
"line": 79,
"line": 81,
"column": 14
}
},
Expand Down Expand Up @@ -661,11 +661,11 @@
"highlighting": "string",
"loc": {
"start": {
"line": 104,
"line": 103,
"column": 6
},
"end": {
"line": 104,
"line": 103,
"column": 74
}
},
Expand Down Expand Up @@ -709,11 +709,11 @@
"highlighting": "string",
"loc": {
"start": {
"line": 49,
"line": 55,
"column": 6
},
"end": {
"line": 49,
"line": 55,
"column": 67
}
},
Expand All @@ -725,11 +725,11 @@
"highlighting": "string",
"loc": {
"start": {
"line": 55,
"line": 67,
"column": 6
},
"end": {
"line": 55,
"line": 67,
"column": 72
}
},
Expand All @@ -741,11 +741,11 @@
"highlighting": "string",
"loc": {
"start": {
"line": 61,
"line": 79,
"column": 6
},
"end": {
"line": 61,
"line": 79,
"column": 68
}
},
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ const slugify = str => {
const createExample = example => {
if (!example) {
throw new Error(
`One of your example pages is undefined. This usually happens when you export or import it with the wrong name.`
'One of your example pages is undefined. This usually happens when you export or import it with the wrong name.'
);
}

Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/card/card_image.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default () => (
href="https://elastic.github.io/eui/"
image="https://source.unsplash.com/400x200/?City"
icon={<EuiIcon size="xxl" type="logoBeats" />}
title={`Beats in the City`}
title={'Beats in the City'}
description="This card has an href and should be a link."
/>
</EuiFlexItem>
Expand Down
Loading

0 comments on commit 190e6b4

Please sign in to comment.