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

ESLint Config Migration: Add test-specific ruleset #1044

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Aug 6, 2021

Summary

This is a PR that should be merged into #1024 and incrementally addresses #842.

This PR adds test-code-specific rules:

  • Turns off the prefer-arrow-callback rule - but only for test files (i.e. **/*.spec.ts). The use of function over arrow syntax actually came up when writing tests in one time in Mocha (see Add tests for SocketModeReceiver (#750) #1012 (review)). As such, we shouldn't have the linter recommend to use only arrow functions in test files, as that may lead to the same situation as in Add tests for SocketModeReceiver (#750) #1012.
  • Loosens up how we do type assertions in code (via the consistent-type-assertions rule), to allow for the const x = { ... } as T over the const x: T = { ... } syntax. The latter requires all properties of a type to be declared, whereas the former requires only the required properties to be defined. I believe the as T syntax is handy to have in test code.
  • Turns off the symbol-descriptor rule and allows for creating Symbol objects without a descriptor.

Impact

Before

✖ 362 problems (173 errors, 189 warnings)
  76 errors and 0 warnings potentially fixable with the `--fix` option.

After

✖ 330 problems (141 errors, 189 warnings)
  64 errors and 0 warnings potentially fixable with the `--fix` option.

Requirements (place an x in each [ ])

@filmaj filmaj added the tests M-T: Testing work only label Aug 6, 2021
@filmaj filmaj self-assigned this Aug 6, 2021
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

💯 comment for reasoning 👍

@filmaj filmaj changed the title Add test-specific ruleset and disable prefer-arrow-callback for tests. Add test-specific ruleset Aug 9, 2021
@filmaj filmaj force-pushed the disable-arrow-callback-tests branch from 46ec80b to 4723cc4 Compare August 9, 2021 16:19
@filmaj filmaj changed the title Add test-specific ruleset ESLint Config Migration: Add test-specific ruleset Aug 9, 2021
@filmaj filmaj force-pushed the disable-arrow-callback-tests branch 2 times, most recently from 59e4a06 to 9b348fe Compare August 9, 2021 23:56
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

approved again

@filmaj
Copy link
Contributor Author

filmaj commented Aug 10, 2021

Thanks for your review @seratch; FYI I added two more rule changes (reminder: that only apply to **/*.spec.ts files) since when you reviewed this PR last time:

  1. Loosen up how we do type assertions in code (via the consistent-type-assertions rule), to allow for the const x = { ... } as T over the const x: T = { ... } syntax. The latter requires all properties of a type to be declared, whereas the former requires only the required properties to be defined. I believe the as T syntax is handy to have in test code (to e.g. test partial object structures depending on the feature under test).
  2. Turns off the symbol-descriptor rule and allows for creating Symbol objects without a descriptor.

If you could let me know what you think about those, I would very much appreciate it 🙏

EDIT: 😆 @seratch nevermind you are too fast for me 😂

@seratch
Copy link
Member

seratch commented Aug 10, 2021

@filmaj Both look good to me 👍

.eslintrc.js Outdated Show resolved Hide resolved
Allow use of "as Type" syntax for type assertions in test code.

Dont force symbol descriptors in test code
@filmaj filmaj force-pushed the disable-arrow-callback-tests branch from 9b348fe to 7b6be5d Compare August 23, 2021 17:37
@filmaj filmaj merged commit ab0b6ab into tslint-to-eslint Aug 23, 2021
@filmaj filmaj deleted the disable-arrow-callback-tests branch August 23, 2021 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests M-T: Testing work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants