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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update linting rules #351

Merged
merged 4 commits into from Jul 25, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions .eslintrc.js
Expand Up @@ -25,6 +25,10 @@ module.exports = {
es6: true,
},
rules: {
'@typescript-eslint/array-type': ['error', 'array-simple'],
Copy link
Member

Choose a reason for hiding this comment

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

In Jest we have this set to generic for consistency

Copy link
Member

Choose a reason for hiding this comment

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

consistency with what? (I don't care either way, and since this is fixable it really doesn't matter what we go with, as long as we lint for one or the other 馃檪)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My two-cents: I've found array-simple to look rather nice, and gives you the best of both worlds in terms of readability vs extra typing/characters/line length.

Just for the record, here's the difference:

// 'generic' - always use "Array", never "[]"
declare const arr: Array<string | number>;
declare const arr: Array<MyObj<string>>;
declare const arr: Array<string>;

// 'array-simple' - always use '[]', unless the generic type is "complex" (uses generics, union, etc)
declare const arr: Array<string | number>;
declare const arr: Array<MyObj<string>>;
declare const arr: string[];

// 'simple' - always use `[]`, never `Array`
declare const arr: (string | number)[];
declare const arr: MyObj<string>[];
declare const arr: string[];

I use to use generic, but got sick of having to go to the start of the type everytime I wanted to make thing like a string an array - w/ array-simple you can just keep typing when doing quick speedy type definitions.

Copy link
Member

Choose a reason for hiding this comment

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

Just thought it'd be nice to have a code base that is easy to get into if you've already worked on Jest itself. Don't care much either, I always write it in array and let it autofix anyway 馃槃

'@typescript-eslint/no-require-imports': 'error',
'@typescript-eslint/ban-ts-ignore': 'warn',
'no-else-return': 'error',
eqeqeq: ['error', 'smart'],
strict: 'error',
'prefer-template': 'warn',
Expand All @@ -42,6 +46,12 @@ module.exports = {
'import/no-extraneous-dependencies': 'error',
},
overrides: [
{
files: ['*.js'],
rules: {
'@typescript-eslint/no-require-imports': 'off',
},
},
{
files: ['*.test.js', '*.test.ts'],
globals,
Expand Down
2 changes: 1 addition & 1 deletion src/rules/no-if.ts
Expand Up @@ -34,7 +34,7 @@ export default createRule({
},
defaultOptions: [],
create(context) {
const stack: Array<boolean> = [];
const stack: boolean[] = [];

function validate(
node: TSESTree.ConditionalExpression | TSESTree.IfStatement,
Expand Down
3 changes: 1 addition & 2 deletions src/rules/no-large-snapshots.js
Expand Up @@ -35,9 +35,8 @@ const reportOnViolation = (context, node) => {
isWhitelisted = whitelistedSnapshotsInFile.some(name => {
if (name.test && typeof name.test === 'function') {
return name.test(snapshotName);
} else {
return name === snapshotName;
}
return name === snapshotName;
});
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/rules/valid-expect-in-promise.js
Expand Up @@ -14,9 +14,8 @@ const isExpectCallPresentInFunction = body => {
return isExpectCall(line.expression);
if (line.type === 'ReturnStatement') return isExpectCall(line.argument);
});
} else {
return isExpectCall(body);
}
return isExpectCall(body);
};

const isExpectCall = expression => {
Expand Down