-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat!: Upgrade @typescript-eslint dependencies #6
Conversation
i think for both questions answer is yes, boolean sounds good and banning object instead of record seems okay (but need to check how many erros we have) |
@Nikamura, we could try banning |
yup, incremental changes |
Follow-up tickets created for both questions: |
BREAKING CHANGE: Dependencies updated to require @typescript-eslint v3.0.0.
https://github.com/typescript-eslint/typescript-eslint/releases/tag/v3.0.0
Changes summary
recommended
andrecommended-requiring-type-checking
configs now extendeslint-recommended
(no need to extendeslint-recommended
).ban-types rule now has very sensible defaults (which contain the stuff we already had).
generic-type-naming
andinterface-name-prefix
are now part of naming-convention. It was quite tricky to get it right so it wouldn't throw errors against canary.relevant disabled/enabled rules which were removed/added from/to recommended configs were removed or added where appropriate
Questions
object
?One of the entries in the new defaults for the
ban-types
iswhich makes sense, although I expect it to possibly cause trouble. Some of our published packages use
object
type and it's hard to determine the impact of this rule on projects using them if it's introduced.canary test run results
Upgrade preview: https://github.com/deepcrawl/canary/compare/style/eslint-config?expand=1
(no linting errors with the currently proposed configuration)
The following rules were determined to be faulty, resulting in false-positive errors, and had to be disabled:
@typescript-eslint/no-unsafe-assignment
e.g.
const app = new cdk.App();
is considered "Unsafe assignment of an any value"@typescript-eslint/no-unsafe-call
e.g.
new CodebuildStack(app, "CanaryCodebuildStack")
is considered "Unsafe construction of an any type value"@typescript-eslint/no-unsafe-member-access
e.g.
cdk.Tag.add(stack, "Team", "Ziggy")
is considered "Unsafe member access .add on an any value"@typescript-eslint/no-unsafe-return
e.g.
localStorage.getItem
return type isstring | null
but in the snippet belowreturn
is still consideredany
by the rule@typescript-eslint/restrict-template-expressions
e.g. works incorrectly with template literals in factories for tests or when literal is using generic type
Additionally,
@typescript-eslint/explicit-module-boundary-types
was disabled. The default recommended setting waswarn
but it seems to incorrectly report when types can be inferred.