Turn off @typescript-eslint/no-redundant-type-constituents #1861
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change removes inline ESLint disable comments for the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
💻 FW Lite
|
The latest updates on your projects. Learn more about Argos notifications ↗︎ Awaiting the start of a new Argos build… |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/eslint.config.js (1)
41-46: Document the temporary rule disable to avoid long-term driftSwitching
@typescript-eslint/no-redundant-type-constituentsoff for every Svelte file is a pragmatic way to silence today’s false-positives, but it also removes legitimate signal for future code.
Add a short comment (or a TODO dated ticket number) explaining why it is off and the plan for revisiting/re-enabling it once the underlying Svelte-parser issue is fixed. This prevents the rule from remaining silently disabled forever.frontend/src/lib/components/modals/FormModal.svelte (1)
60-67: Prefer the?optional parameter over| undefinedfor readabilityUsing the
?syntax communicates intent more clearly and drops the now-unnecessaryundefinedconstituent the rule used to complain about.-export async function open( - value: Partial<FormType> | undefined, +export async function open( + value?: Partial<FormType>, onSubmit: SubmitCallback, ): Promise<FormModalResult<Schema>>; // implementation overload -export async function open( - valueOrOnSubmit: Partial<FormType> | SubmitCallback | undefined, +export async function open( + valueOrOnSubmit?: Partial<FormType> | SubmitCallback, _onSubmit?: SubmitCallback,This keeps the API surface identical (
open(undefined, cb)still type-checks) while making the signatures shorter and side-steps the redundant-constituent smell even with the rule disabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
frontend/eslint.config.js(1 hunks)frontend/src/lib/components/FilterBar/FilterBar.svelte(0 hunks)frontend/src/lib/components/Projects/ProjectConfidentialityFilterSelect.svelte(0 hunks)frontend/src/lib/components/modals/ConfirmDeleteModal.svelte(0 hunks)frontend/src/lib/components/modals/FormModal.svelte(1 hunks)frontend/src/routes/(authenticated)/admin/+page.svelte(0 hunks)frontend/src/routes/(authenticated)/admin/EditUserAccount.svelte(0 hunks)frontend/src/routes/(authenticated)/org/[org_id]/AddMyProjectsToOrgModal.svelte(0 hunks)frontend/src/routes/(authenticated)/org/[org_id]/AddOrgMemberModal.svelte(0 hunks)frontend/src/routes/(authenticated)/org/[org_id]/BulkAddOrgMembers.svelte(0 hunks)frontend/src/routes/(authenticated)/org/[org_id]/ChangeOrgMemberRoleModal.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/AddOrganization.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/AddProjectMember.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/AddPurpose.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/BulkAddProjectMembers.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/ChangeMemberRoleModal.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/ProjectConfidentialityModal.svelte(0 hunks)frontend/src/routes/(authenticated)/project/[project_code]/ResetProjectModal.svelte(0 hunks)frontend/viewer/src/lib/OpenInFieldWorksButton.svelte(0 hunks)frontend/viewer/src/lib/components/field-editors/multi-select.svelte(0 hunks)frontend/viewer/src/lib/components/field-editors/select.svelte(0 hunks)frontend/viewer/src/lib/components/ui/icon/pinging-icon.svelte(0 hunks)
💤 Files with no reviewable changes (20)
- frontend/src/routes/(authenticated)/org/[org_id]/BulkAddOrgMembers.svelte
- frontend/src/routes/(authenticated)/org/[org_id]/AddMyProjectsToOrgModal.svelte
- frontend/viewer/src/lib/components/ui/icon/pinging-icon.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/AddProjectMember.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/AddPurpose.svelte
- frontend/src/routes/(authenticated)/admin/EditUserAccount.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/ProjectConfidentialityModal.svelte
- frontend/src/routes/(authenticated)/org/[org_id]/ChangeOrgMemberRoleModal.svelte
- frontend/src/lib/components/FilterBar/FilterBar.svelte
- frontend/viewer/src/lib/components/field-editors/select.svelte
- frontend/viewer/src/lib/OpenInFieldWorksButton.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/ResetProjectModal.svelte
- frontend/src/routes/(authenticated)/org/[org_id]/AddOrgMemberModal.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/ChangeMemberRoleModal.svelte
- frontend/src/lib/components/modals/ConfirmDeleteModal.svelte
- frontend/src/lib/components/Projects/ProjectConfidentialityFilterSelect.svelte
- frontend/src/routes/(authenticated)/admin/+page.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/BulkAddProjectMembers.svelte
- frontend/viewer/src/lib/components/field-editors/multi-select.svelte
- frontend/src/routes/(authenticated)/project/[project_code]/AddOrganization.svelte
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rmunn
PR: sillsdev/languageforge-lexbox#1836
File: frontend/viewer/src/lib/components/audio/AudioDialog.svelte:25-25
Timestamp: 2025-07-22T09:19:37.386Z
Learning: In the sillsdev/languageforge-lexbox project, when file size limits or other constants need to be shared between C# backend and TypeScript frontend code, prefer exposing them through Reinforced.Typings type generation rather than hardcoding the values separately. This ensures consistency and prevents discrepancies when values change.
frontend/eslint.config.js (1)
Learnt from: rmunn
PR: #1836
File: frontend/viewer/src/lib/components/audio/AudioDialog.svelte:25-25
Timestamp: 2025-07-22T09:19:37.386Z
Learning: In the sillsdev/languageforge-lexbox project, when file size limits or other constants need to be shared between C# backend and TypeScript frontend code, prefer exposing them through Reinforced.Typings type generation rather than hardcoding the values separately. This ensures consistency and prevents discrepancies when values change.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build UI / publish-ui
- GitHub Check: Build API / publish-api
- GitHub Check: check-and-lint
- GitHub Check: frontend-component-unit-tests
- GitHub Check: frontend
- GitHub Check: Analyze (csharp)
- GitHub Check: Build FW Lite and run tests
imnasnainaec
force-pushed
the
no-redundant-type-constituents
branch
from
c5d63dc to
f3a6542
Compare
|
I'd like to put this on hold as I don't actually want to share linting rules between the extension and the rest of our code. See #1831 (comment) |
@hahn-kev That's a good "want", which I'm working on separately. I thought this pr might have its own merits independent of the extension. You are welcome to merge it or close it as you will. |
|
@myieye and @rmunn what do you think about just disabling this rule? Here's the docs https://typescript-eslint.io/rules/no-redundant-type-constituents/ |
|
In our team meeting we would like to keep this lint rule, the places where it was disabled previously were false positives (per Tim), so it's worth checking if that's still the case then we can remove the comments disabling the rule. |
|
To complete what @hahn-kev said, I think we're probably ok with this PR, if after the es-lint updates/package bumps @rmunn recently made, svelte files are still riddled with these false positives. At least I'm pretty sure they're false positives. It does seem weird that Svelte tooling would still be struggling with these. |
imnasnainaec
force-pushed
the
no-redundant-type-constituents
branch
from
f3a6542 to
8e83297
Compare
@myieye Yes, they're all still getting hit and they look like false positives to me. |
frontend/src/routes/(authenticated)/project/[project_code]/+page.ts
Outdated
Show resolved
Hide resolved
frontend/src/routes/(authenticated)/project/[project_code]/+page.ts
Outdated
Show resolved
Hide resolved
3 participants
The false positives for
'@typescript-eslint/no-redundant-type-constituents'appear related to the other 5 rules disabled for*.sveltefiles.These 6 rules are also the source of most noise on theimprove-platform-extensionbranch, so this pr prepares for re-enabling linting infrontend/platform.bible-extension/on that branch.