-
Notifications
You must be signed in to change notification settings - Fork 561
build(eslint-config-fluid): update @typescript-eslint to v8 #25663
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
build(eslint-config-fluid): update @typescript-eslint to v8 #25663
Conversation
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
Updates the TypeScript ESLint plugin from v7.18.0 to v8.18.2, adapting configuration to handle breaking changes in the new major version. The update removes deprecated rules and introduces new ones for better TypeScript code quality.
Key changes:
- Upgrade TypeScript ESLint plugin and parser to v8.18.2
- Replace deprecated
no-throw-literalwithonly-throw-errorrule - Remove deprecated
ban-typesandno-empty-interfacerules and their disables - Add
no-restricted-typesrule to discourage null usage in favor of undefined
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| package.json | Updates TypeScript ESLint dependencies to v8.18.2 |
| base.js | Removes deprecated rule configurations and adds new v8-compatible rules |
Files not reviewed (1)
- common/build/eslint-config-fluid/pnpm-lock.yaml: Language not supported
0c485e8 to
286a85b
Compare
| // FIXME: Is this the right fix? There are lots of errors here. | ||
| "@fluid-internal/fluid/no-unchecked-record-access": "off", | ||
|
|
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.
@jason-ha This rule seems to be triggered on a lot of presence code with the typescript-eslint upgrade in this PR. My hunch is they're false positives, but I didn't look closely. I am not inclined to do a deep investigation on most issues until we're on a stable/supported version of the stack. But since this is a custom rule, it could also be a bug there, or something else.
I could also make them warnings for now instead of disabling, so they could be investigated more deeply. Or if you think a deeper investigation is warranted and should be blocking, I'm open to discussing.
@Josmithr FYI.
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.
Last I recall, that rule still had lots of issues and is disabled in a lot of places across the repo. Could use some investment, I think.
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Updates @typescript-eslint packages from v7 to v8 with necessary rule migrations and configuration adjustments.
Changes
no-throw-literal→only-throw-errorban-types,no-empty-interface, and othersTest plan