Skip to content

Added specific eslint rules for typescript tests#2634

Closed
dotansimha wants to merge 2 commits intographql:ts_testsfrom
dotansimha:ts-lint-rules
Closed

Added specific eslint rules for typescript tests#2634
dotansimha wants to merge 2 commits intographql:ts_testsfrom
dotansimha:ts-lint-rules

Conversation

@dotansimha
Copy link
Copy Markdown
Member

@dotansimha dotansimha commented Jun 10, 2020

Extracted from: #2609

@IvanGoncharov

@dotansimha dotansimha marked this pull request as ready for review June 10, 2020 12:05
@dotansimha dotansimha changed the title dded specific eslint rules for typescript tests Added specific eslint rules for typescript tests Jun 10, 2020
Comment thread .eslintrc.yml
Comment thread .eslintrc.yml
'@typescript-eslint/no-throw-literal': off
'@typescript-eslint/require-await': off
'@typescript-eslint/await-thenable': off
'prefer-promise-reject-errors': 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also please use off

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No all rules allow off as value, it gives an error. I think only TS rules allow off value.
image

Comment thread .eslintrc.yml
'@typescript-eslint/require-await': off
'@typescript-eslint/await-thenable': off
'prefer-promise-reject-errors': 0
'no-new-wrappers': 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are standard ESLint rules so they should work on both Flow and TS.
Can you please give an example of why you disabled them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Some of the tests are creating wrapped instances of primitives (for example - isCollection-test.js).
At the moment it has // eslint-disable-next-line no-new-wrappers set on those lines, but isn't it better to disable it on a global level? there are multiple places that does that.

@dotansimha
Copy link
Copy Markdown
Member Author

@IvanGoncharov ping :)

@Urigo Urigo mentioned this pull request Sep 26, 2020
6 tasks
@dotansimha dotansimha mentioned this pull request Dec 1, 2020
17 tasks
@IvanGoncharov IvanGoncharov deleted the branch graphql:ts_tests June 20, 2021 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants