Skip to content

Conversation

dotansimha
Copy link
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
'@typescript-eslint/no-throw-literal': off
'@typescript-eslint/require-await': off
'@typescript-eslint/await-thenable': off
'prefer-promise-reject-errors': 0
Copy link
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
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

'@typescript-eslint/require-await': off
'@typescript-eslint/await-thenable': off
'prefer-promise-reject-errors': 0
'no-new-wrappers': 0
Copy link
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
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
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