-
Notifications
You must be signed in to change notification settings - Fork 559
build(eslint-config-fluid): migrate @rushstack/no-new-null to @typescript-eslint/no-restricted-types #25664
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
base: main
Are you sure you want to change the base?
build(eslint-config-fluid): migrate @rushstack/no-new-null to @typescript-eslint/no-restricted-types #25664
Conversation
…ript-eslint/no-restricted-types Replace the Rushstack rule with the equivalent TypeScript ESLint rule as recommended by the TypeScript ESLint migration guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates from the Rushstack ESLint plugin rule @rushstack/no-new-null to the equivalent TypeScript ESLint rule @typescript-eslint/no-restricted-types as part of a broader effort to migrate away from Rushstack plugins. The migration maintains the same functionality of preventing the use of null in favor of undefined.
- Replaces
@rushstack/no-new-nullwith@typescript-eslint/no-restricted-typesconfigured to restrictnullusage - Updates both the recommended and minimal-deprecated ESLint configurations
- Maintains the same error/warning levels and provides equivalent functionality with auto-fix support
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| common/build/eslint-config-fluid/recommended.js | Migrates the rule from Rushstack to TypeScript ESLint with "error" severity |
| common/build/eslint-config-fluid/minimal-deprecated.js | Migrates the rule from Rushstack to TypeScript ESLint with "warn" severity and updates documentation |
alexvy86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only quick glance so far: I see several new disables. Double checking that they are legit new hits, like we know can happen from more up-to-date rules, and not copilot pattern-matching on places that might have had a package-level disable or similar?
Good question. The old rushstack no-new-null rule apparently is not as sensitive as the replacement rule. At first I thought that was a good thing, but because we have so many types that go over the wire, we have a lot of legitimate null usage. Anyway, I originally thought the rushstack replacement was needed because it wasn't compatible with eslint9. But I was wrong about that, so I am leaning towards just upgrading it and postponing any further related work. |
Replace the Rushstack rule with the equivalent TypeScript ESLint rule
as recommended by the TypeScript ESLint migration guide.
Blocked by
Blocking