Skip to content

feat: migrate off of tslint and use eslint#777

Open
parker-snyk wants to merge 6 commits intomainfrom
CN-934-tslint-to-eslint
Open

feat: migrate off of tslint and use eslint#777
parker-snyk wants to merge 6 commits intomainfrom
CN-934-tslint-to-eslint

Conversation

@parker-snyk
Copy link
Copy Markdown
Contributor

What does this PR do?

TSLint has been deprecated since 2019.
This PR migrates us to ESLint, which is still supported, and has more features and support

image

Any background context you want to provide?

This shouldn't have any downsteam effects for anyone using this library. It just lets us lint better and move faster for development on this project

What are the relevant tickets?

https://snyksec.atlassian.net/browse/CN-934

Screenshots

@parker-snyk parker-snyk requested a review from a team as a code owner April 6, 2026 14:25
@parker-snyk parker-snyk requested a review from kateeselius April 6, 2026 14:25
@snyk-pr-review-bot

This comment has been minimized.

@parker-snyk parker-snyk force-pushed the CN-934-tslint-to-eslint branch from 58c06dc to ac092c7 Compare April 6, 2026 14:31
@snyk snyk deleted a comment from snyk-pr-review-bot bot Apr 6, 2026
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

The eslint migration accidentally removed the variable assignment for
depGraph in a test that still uses it in assertions, causing a
ReferenceError at runtime.
@parker-snyk parker-snyk force-pushed the CN-934-tslint-to-eslint branch from d5436e1 to 7b88c17 Compare April 6, 2026 19:39
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@parker-snyk parker-snyk force-pushed the CN-934-tslint-to-eslint branch from 794d9fa to 58c5b56 Compare April 10, 2026 13:24
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Disabled Safety Rules 🟡 [minor]

The ESLint configuration explicitly disables several important rules, such as no-shadow, @typescript-eslint/no-shadow, and @typescript-eslint/no-explicit-any. While this might align with previous TSLint configurations, no-shadow helps prevent variable naming conflicts that can lead to subtle bugs, and @typescript-eslint/no-explicit-any is crucial for maintaining type safety in a TypeScript codebase. Re-enabling or carefully considering these rules could improve code quality and prevent future issues, rather than relying solely on developer vigilance.

"no-shadow": "off",
"@typescript-eslint/no-shadow": "off",
"@typescript-eslint/naming-convention": "off",
"no-bitwise": "off",
"max-classes-per-file": "off",
"no-console": "off",
"@typescript-eslint/no-explicit-any": "off",
"no-case-declarations": "off",
"no-useless-escape": "off",
"no-prototype-builtins": "off",
"@typescript-eslint/no-require-imports": "off",
"@typescript-eslint/no-namespace": "off",
"no-control-regex": "off",
Loose Unused Var Check 🟡 [minor]

The @typescript-eslint/no-unused-vars rule is configured with caughtErrors: "none". This setting means that caught error variables will not be flagged as unused, even if they are genuinely not used within the catch block. While this might be intended to avoid linting errors for empty catch blocks, it can hide cases where an error object is passed but not utilized, potentially leading to missed debugging opportunities or silent failures. Consider setting caughtErrors to 'all' or 'auto' if there's an expectation that caught errors should be handled or logged.

caughtErrors: "none",
📚 Repository Context Analyzed

This review considered 21 relevant code sections from 13 files (average relevance: 0.87)

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.

1 participant