fix: remove unnecessary ternary expressions and map spreads#8750
Open
darkingtail wants to merge 3 commits intomakeplane:previewfrom
Open
fix: remove unnecessary ternary expressions and map spreads#8750darkingtail wants to merge 3 commits intomakeplane:previewfrom
darkingtail wants to merge 3 commits intomakeplane:previewfrom
Conversation
Resolve oxlint warnings: - no-unneeded-ternary: simplify redundant boolean ternaries - no-map-spread: replace inefficient spread-in-map patterns
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (32)
🚧 Files skipped from review as they are similar to previous changes (25)
📝 WalkthroughWalkthroughRefactors many files to simplify boolean expressions (removing redundant ternaries, using !!/??) and replaces several object spread patterns with Object.assign; also updates numerous store types from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (23)
apps/web/core/components/modules/dropdowns/filters/start-date.tsx-69-69 (1)
69-69:⚠️ Potential issue | 🟡 MinorPreserve a strict boolean for
isChecked.This simplification is not equivalent to the previous ternary: when
appliedFiltersis nullish, Line 69 now passesundefinedinstead offalse. IfFilterOptiontreats “unset” differently fromfalse, this changes behavior.Suggested fix
- isChecked={appliedFilters?.includes(option.value)} + isChecked={appliedFilters?.includes(option.value) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/modules/dropdowns/filters/start-date.tsx` at line 69, The isChecked prop currently can become undefined when appliedFilters is nullish; change the expression used for isChecked (in the component rendering the FilterOption in start-date.tsx) to always produce a strict boolean—e.g., coerce appliedFilters?.includes(option.value) to true/false using a boolean cast or a conditional that returns false when appliedFilters is nullish—so FilterOption receives either true or false, not undefined.packages/editor/src/core/extensions/slash-commands/command-items-list.tsx-324-325 (1)
324-325:⚠️ Potential issue | 🟡 MinorReturn a boolean from this
filtercallback.Line 325 returns
undefined, which is what Biome is complaining about here. Sincequeryis already typed asstring, the simplest fix is to remove the guard; otherwise returnfalseexplicitly.Suggested fix
const filteredSlashSections = SLASH_COMMAND_SECTIONS.map((section) => Object.assign({}, section, { items: section.items.filter((item) => { - if (typeof query !== "string") return; - const lowercaseQuery = query.toLowerCase(); return ( item.title.toLowerCase().includes(lowercaseQuery) || item.description.toLowerCase().includes(lowercaseQuery) || item.searchTerms.some((t) => t.includes(lowercaseQuery)) ); }), }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/core/extensions/slash-commands/command-items-list.tsx` around lines 324 - 325, The filter callback for section.items.filter currently can return undefined because of the guard "if (typeof query !== 'string') return;", so update the callback in command-items-list.tsx to always return a boolean: either remove the unnecessary typeof-guard (since query is typed as string) or change it to "if (typeof query !== 'string') return false;". Ensure the callback used by items: section.items.filter((item) => { ... }) returns true/false in all code paths so Biome no longer flags an undefined return.apps/web/core/components/project-states/options/delete.tsx-35-35 (1)
35-35:⚠️ Potential issue | 🟡 MinorRemove the remaining redundant ternary.
Line 35 still matches the
no-unneeded-ternarypattern this PR is trying to eliminate, so it may keep the lint warning alive.Suggested change
- const isDeleteDisabled = state.default ? true : totalStates === 1; + const isDeleteDisabled = state.default || totalStates === 1;As per coding guidelines, "Use OxLint with shared
.oxlintrc.jsonconfig for linting".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/project-states/options/delete.tsx` at line 35, The expression for isDeleteDisabled uses an unnecessary ternary; simplify the constant by assigning the boolean expression directly (use state.default || totalStates === 1) so replace the ternary in the isDeleteDisabled declaration with the equivalent boolean expression to satisfy the no-unneeded-ternary lint rule and keep state.default and totalStates references intact.apps/space/components/issues/filters/priority.tsx-48-48 (1)
48-48:⚠️ Potential issue | 🟡 MinorApply boolean coercion to fix type mismatch in
isCheckedprop.
appliedFiltershas typestring[] | null(line 18), soappliedFilters?.includes(priority.key)returnsboolean | undefined. TheFilterOptioncomponent requiresisChecked: boolean(strict boolean). Using!!coercion ensures the correct type:🛠️ Proposed fix
<FilterOption key={priority.key} - isChecked={appliedFilters?.includes(priority.key)} + isChecked={!!appliedFilters?.includes(priority.key)} onClick={() => handleUpdate(priority.key)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/space/components/issues/filters/priority.tsx` at line 48, The isChecked prop on FilterOption is receiving type boolean | undefined because appliedFilters has type string[] | null; update the isChecked expression to coerce to a strict boolean (e.g., use double-bang) so it always passes a boolean: change the current isChecked={appliedFilters?.includes(priority.key)} to a boolean-coerced expression using appliedFilters and priority.key so FilterOption.isChecked receives a boolean value.apps/web/core/components/project/dropdowns/filters/lead.tsx-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed to prop expectingboolean.Since
appliedFiltersis typed asstring[] | null, the expressionappliedFilters?.includes(member.id)returnsboolean | undefined, notboolean. TheFilterOptioncomponent'sisCheckedprop is strictly typed asboolean.While this works at runtime (undefined is falsy), it violates the TypeScript type contract. Use double negation to coerce to boolean:
🛠️ Proposed fix
<FilterOption key={`lead-${member.id}`} - isChecked={appliedFilters?.includes(member.id)} + isChecked={!!appliedFilters?.includes(member.id)} onClick={() => handleUpdate(member.id)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/project/dropdowns/filters/lead.tsx` at line 77, The isChecked prop on FilterOption is receiving a boolean | undefined because appliedFilters is string[] | null; coerce the expression appliedFilters?.includes(member.id) to a strict boolean (e.g. using double-negation or Boolean()) so isChecked always gets a boolean; update the usage of appliedFilters?.includes(member.id) inside the FilterOption prop to !!appliedFilters?.includes(member.id) (or equivalent) to satisfy the prop type.apps/space/components/issues/filters/labels.tsx-59-59 (1)
59-59:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop withstrictNullChecksenabled.Per
apps/space/tsconfig.json,strictNullChecksistrue. TheFilterOptioncomponent'sisCheckedprop is typed asboolean(notboolean | undefined), butappliedFilters?.includes(label?.id)returnsundefinedwhenappliedFiltersisnull. This should cause a TypeScript error.Proposed fix
- isChecked={appliedFilters?.includes(label?.id)} + isChecked={appliedFilters?.includes(label?.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/space/components/issues/filters/labels.tsx` at line 59, The isChecked prop of FilterOption is receiving a boolean | undefined because appliedFilters?.includes(label?.id) can be undefined; change the expression to guarantee a boolean (for example by using a boolean coercion or defaulting appliedFilters to an empty array) so isChecked always receives a boolean; update the usage in labels.tsx where appliedFilters and label?.id are referenced (the FilterOption isChecked prop) to use a deterministic boolean (e.g., !!appliedFilters?.includes(label?.id) or (appliedFilters ?? []).includes(label?.id)).apps/web/core/components/issues/issue-layouts/filters/header/filters/labels.tsx-71-71 (1)
71-71:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.
appliedFilters?.includes(label?.id)returnsundefinedwhenappliedFiltersisnull. Add nullish coalescing for type correctness.Proposed fix
- isChecked={appliedFilters?.includes(label?.id)} + isChecked={appliedFilters?.includes(label?.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/labels.tsx` at line 71, The isChecked prop is receiving a boolean | undefined because appliedFilters?.includes(label?.id) can be undefined when appliedFilters is null; update the expression used for isChecked (in the Labels component where isChecked={appliedFilters?.includes(label?.id)}) to coerce to a boolean (e.g., use nullish coalescing or double-bang) so it always passes a boolean (for example replace with appliedFilters?.includes(label?.id) ?? false or !!appliedFilters?.includes(label?.id)).apps/web/core/components/modules/dropdowns/filters/status.tsx-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.When
appliedFiltersisnull, the expression returnsundefined. Add nullish coalescing for consistency with the expectedbooleantype.Proposed fix
- isChecked={appliedFilters?.includes(status.value)} + isChecked={appliedFilters?.includes(status.value) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/modules/dropdowns/filters/status.tsx` at line 44, The isChecked prop for the checkbox currently receives appliedFilters?.includes(status.value) which can be boolean | undefined when appliedFilters is null; change it to a guaranteed boolean by using nullish coalescing on the includes result (e.g., appliedFilters?.includes(status.value) ?? false) so isChecked always receives a boolean; update the usage where isChecked is passed (the component rendering the checkbox, referencing appliedFilters and status.value) to use this coalesced expression.apps/web/core/components/project/dropdowns/filters/members.tsx-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Same issue as other filter components—
appliedFilters?.includes(member.id)can beundefinedwhenappliedFiltersisnull, butisCheckedrequiresboolean.Proposed fix
- isChecked={appliedFilters?.includes(member.id)} + isChecked={appliedFilters?.includes(member.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/project/dropdowns/filters/members.tsx` at line 77, The isChecked prop is receiving a boolean | undefined because appliedFilters?.includes(member.id) can be undefined when appliedFilters is null; update the expression where isChecked is set (in the Members filter component using appliedFilters and member.id) to always produce a boolean—for example coerce with !!appliedFilters?.includes(member.id) or use (appliedFilters ?? []).includes(member.id)—so isChecked receives a strict boolean value.apps/web/core/components/issues/issue-layouts/filters/header/filters/state.tsx-63-63 (1)
63-63:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.
appliedFilters?.includes(state.id)returnsundefinedwhenappliedFiltersisnull, butFilterOption'sisCheckedprop expects a strictboolean. Add a nullish coalescing operator to ensure a boolean value.Proposed fix
- isChecked={appliedFilters?.includes(state.id)} + isChecked={appliedFilters?.includes(state.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/state.tsx` at line 63, The isChecked prop on FilterOption is receiving a boolean | undefined from appliedFilters?.includes(state.id); update the prop to always pass a boolean by coalescing the result to false (e.g., use appliedFilters?.includes(state.id) ?? false or wrap with !!) so FilterOption.isChecked receives a strict boolean; change the usage where isChecked={appliedFilters?.includes(state.id)} to use the nullish/coercion fix referencing appliedFilters and state.id.apps/web/core/components/modules/dropdowns/filters/lead.tsx-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.When
appliedFiltersisnull, optional chaining returnsundefined. TheFilterOptioncomponent'sisCheckedprop expects a strictbooleantype (seefilter-option.tsxline 11). Add nullish coalescing to ensure type safety.Proposed fix
- isChecked={appliedFilters?.includes(member.id)} + isChecked={appliedFilters?.includes(member.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/modules/dropdowns/filters/lead.tsx` at line 77, The isChecked prop on FilterOption is receiving a boolean | undefined because appliedFilters?.includes(member.id) can be undefined when appliedFilters is null; change the expression used where isChecked is set (currently appliedFilters?.includes(member.id)) to coerce a strict boolean (for example via nullish coalescing or Boolean()) so isChecked always receives a boolean value; update the usage near the FilterOption invocation referencing appliedFilters and member.id to return false when appliedFilters is null.apps/web/core/components/issues/issue-layouts/filters/header/filters/start-date.tsx-63-66 (1)
63-66:⚠️ Potential issue | 🟡 MinorKeep
isCheckedboolean on thenullpath.
appliedFiltersis nullable, so this now passesundefinedwhen filters haven't been initialized. The old ternary always passedfalse, which is closer to the PR's “no behavioral change” goal and avoids leaking a tri-state value intoFilterOption.As per coding guidelines, `**/*.{ts,tsx}`: Enable TypeScript strict mode and ensure all files must be typed.💡 Suggested fix
- isChecked={appliedFilters?.includes(option.value)} + isChecked={!!appliedFilters?.includes(option.value)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/start-date.tsx` around lines 63 - 66, The isChecked prop on FilterOption can become undefined because appliedFilters is nullable; change the expression used in the FilterOption JSX so that isChecked is always a boolean (false when appliedFilters is null/undefined) instead of passing undefined. Locate the FilterOption usage (props: isChecked, onClick with handleUpdate) and replace the current appliedFilters?.includes(option.value) with a boolean-safe check (e.g., (appliedFilters ?? []).includes(option.value) or Boolean(appliedFilters && appliedFilters.includes(option.value))) so isChecked is always true/false.apps/web/core/components/common/filters/created-by.tsx-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Per the
FilterOptiontype definition (context snippet),isCheckedexpects a strictboolean. The optional chaining returnsundefinedwhenappliedFiltersisnull.🔧 Proposed fix
- isChecked={appliedFilters?.includes(member.id)} + isChecked={appliedFilters?.includes(member.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/common/filters/created-by.tsx` at line 77, The isChecked prop for FilterOption is receiving a boolean | undefined due to optional chaining on appliedFilters; change the expression to produce a strict boolean — e.g., replace isChecked={appliedFilters?.includes(member.id)} with a boolean-coerced form like isChecked={!!appliedFilters?.includes(member.id)} or by checking (appliedFilters ?? []).includes(member.id) so FilterOption receives a true/false value.apps/web/core/components/cycles/dropdowns/filters/end-date.tsx-67-67 (1)
67-67:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Consistent with other filter components —
appliedFilters?.includes(option.value)returnsboolean | undefined.🔧 Proposed fix
- isChecked={appliedFilters?.includes(option.value)} + isChecked={appliedFilters?.includes(option.value) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/cycles/dropdowns/filters/end-date.tsx` at line 67, The isChecked prop currently receives appliedFilters?.includes(option.value) which can be boolean | undefined; coerce it to a strict boolean to match the prop type by wrapping the expression with an explicit boolean cast (e.g., using Boolean(...) or !!...) where isChecked={...} is set; update the usage around appliedFilters, option.value and the isChecked prop in end-date.tsx to ensure a plain boolean is passed.apps/web/core/components/issues/issue-layouts/filters/header/filters/priority.tsx-45-45 (1)
45-45:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Same issue as other filter components —
appliedFilters?.includes(priority.key)can returnundefinedwhenappliedFiltersisnull.🔧 Proposed fix
- isChecked={appliedFilters?.includes(priority.key)} + isChecked={appliedFilters?.includes(priority.key) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/priority.tsx` at line 45, The isChecked prop is receiving a boolean | undefined from appliedFilters?.includes(priority.key); change the expression to always produce a boolean (e.g., coalesce appliedFilters to an empty array or force a boolean) so isChecked always gets a boolean; update the usage in the priority.tsx filter component where appliedFilters and priority.key are referenced (the isChecked prop on the checkbox/control) to use a deterministic boolean expression like forcing a boolean or using (appliedFilters ?? []) before calling includes.apps/web/core/components/issues/issue-layouts/filters/header/filters/state-group.tsx-52-52 (1)
52-52:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Consistent with other filter components in this PR — needs explicit boolean coercion.
🔧 Proposed fix
- isChecked={appliedFilters?.includes(stateGroup.key)} + isChecked={appliedFilters?.includes(stateGroup.key) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/state-group.tsx` at line 52, The isChecked prop is receiving a boolean | undefined from appliedFilters?.includes(stateGroup.key); coerce it to a strict boolean (e.g., use !! or Boolean(...)) so isChecked always gets a boolean. Update the expression where isChecked is set (the isChecked prop that references appliedFilters?.includes(stateGroup.key)) to explicitly convert the result to boolean.apps/web/core/components/issues/issue-layouts/filters/header/filters/created-by.tsx-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.
appliedFilters?.includes(member.id)returnsboolean | undefined(whenappliedFiltersisnull), butFilterOption.isCheckedexpects a strictboolean. The original ternary implicitly handled this by coercing tofalse.🔧 Proposed fix using nullish coalescing
- isChecked={appliedFilters?.includes(member.id)} + isChecked={appliedFilters?.includes(member.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/created-by.tsx` at line 77, The isChecked prop on FilterOption is receiving appliedFilters?.includes(member.id), which can be boolean | undefined; change it to a strict boolean by coercing the value (e.g., use nullish coalescing or double-bang) so isChecked always receives a boolean; update the expression where FilterOption.isChecked is set (references: appliedFilters, member.id, FilterOption) to use appliedFilters?.includes(member.id) ?? false or !!appliedFilters?.includes(member.id).apps/space/components/issues/filters/state.tsx-59-59 (1)
59-59:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed toisChecked: booleanprop.When
appliedFiltersisnull, optional chaining returnsundefined, butFilterOptionexpects a strictbooleantype (seefilter-option.tsxline 11). Use nullish coalescing or double negation to ensure a boolean value.🛡️ Proposed fix
- isChecked={appliedFilters?.includes(state.id)} + isChecked={appliedFilters?.includes(state.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/space/components/issues/filters/state.tsx` at line 59, The isChecked prop on FilterOption is receiving appliedFilters?.includes(state.id) which can be boolean | undefined; change it to a strict boolean by coercing the result (e.g., use nullish coalescing or double-negation) so it always passes a boolean to FilterOption; update the expression near isChecked={appliedFilters?.includes(state.id)} to something like isChecked={appliedFilters?.includes(state.id) ?? false} or isChecked={!!appliedFilters?.includes(state.id)} to satisfy the FilterOption prop type.apps/web/core/components/common/filters/created-at.tsx-67-67 (1)
67-67:⚠️ Potential issue | 🟡 MinorAdd nullish coalescing operator to ensure
isCheckedreceivesboolean.The
FilterOptioncomponent expectsisCheckedasboolean, butappliedFilters?.includes(option.value)returnsundefinedwhenappliedFiltersisnull. UseisChecked={appliedFilters?.includes(option.value) ?? false}to provide a default value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/common/filters/created-at.tsx` at line 67, The isChecked prop on FilterOption can receive undefined when appliedFilters is null because appliedFilters?.includes(option.value) yields undefined; change the expression in the FilterOption render (the isChecked prop passed where appliedFilters and option.value are referenced) to use a nullish default so it always gets a boolean (e.g., replace appliedFilters?.includes(option.value) with a nullish-coalesced form to return false when appliedFilters is null).apps/web/core/components/issues/issue-layouts/filters/header/filters/module.tsx-71-71 (1)
71-71:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.When
appliedFiltersisnull, the expressionappliedFilters?.includes(cycle.id)evaluates toundefined, butFilterOption'sisCheckedprop expects a strictboolean. The original ternary served as type coercion.🔧 Proposed fix
- isChecked={appliedFilters?.includes(cycle.id)} + isChecked={appliedFilters?.includes(cycle.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/module.tsx` at line 71, The isChecked prop on FilterOption is receiving a boolean | undefined because appliedFilters?.includes(cycle.id) can be undefined when appliedFilters is null; change the expression to produce a strict boolean (e.g. use !!appliedFilters?.includes(cycle.id) or appliedFilters ? appliedFilters.includes(cycle.id) : false) so isChecked always gets a boolean; update the usage in module.tsx where FilterOption is rendered with isChecked and reference appliedFilters and cycle.id.apps/web/core/components/cycles/dropdowns/filters/start-date.tsx-68-68 (1)
68-68:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Consistent with other filter components, when
appliedFiltersisnull, this evaluates toundefinedrather than a boolean.🔧 Proposed fix
- isChecked={appliedFilters?.includes(option.value)} + isChecked={appliedFilters?.includes(option.value) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/cycles/dropdowns/filters/start-date.tsx` at line 68, The isChecked prop for the checkbox is receiving a boolean | undefined because appliedFilters can be null; update the expression used at isChecked (where appliedFilters?.includes(option.value) is passed) to always produce a boolean (e.g., coerce with !! or Boolean) so isChecked receives a plain boolean; locate the isChecked usage in this component (reference: appliedFilters, option.value, isChecked) and replace the undefined-producing expression with a boolean-coercing one.apps/web/core/components/issues/issue-layouts/filters/header/filters/cycle.tsx-78-78 (1)
78-78:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Same pattern as other filter components—
appliedFilterscan benull, causingundefinedto be passed.🔧 Proposed fix
- isChecked={appliedFilters?.includes(cycle.id)} + isChecked={appliedFilters?.includes(cycle.id) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/cycle.tsx` at line 78, The isChecked prop is receiving a boolean | undefined because appliedFilters can be null; change the expression to always produce a boolean—for example, coerce the result with a double-bang or default appliedFilters to an empty array—so replace isChecked={appliedFilters?.includes(cycle.id)} with a deterministic boolean like isChecked={!!appliedFilters?.includes(cycle.id)} or isChecked={(appliedFilters ?? []).includes(cycle.id)}; update the usage in the component where appliedFilters and cycle.id are referenced to ensure the prop receives a boolean.apps/web/core/components/issues/issue-layouts/filters/header/filters/due-date.tsx-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorType mismatch:
boolean | undefinedpassed tobooleanprop.Same issue as other filter components—when
appliedFiltersisnull, the optional chaining returnsundefinedinstead of a boolean.🔧 Proposed fix
- isChecked={appliedFilters?.includes(option.value)} + isChecked={appliedFilters?.includes(option.value) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/filters/header/filters/due-date.tsx` at line 66, The isChecked prop is receiving a boolean | undefined because appliedFilters can be null; update the expression that sets isChecked (where isChecked={appliedFilters?.includes(option.value)}) to always return a boolean—either coerce with a boolean cast (e.g., !!appliedFilters?.includes(option.value)) or call includes on a safe default (e.g., (appliedFilters ?? []).includes(option.value)); change the code where appliedFilters, option.value and the isChecked prop are used in the due-date filter component so the prop always receives a plain boolean.
🧹 Nitpick comments (12)
apps/web/core/components/cycles/analytics-sidebar/sidebar-details.tsx (1)
53-55: Use strict equality and remove redundant optional chaining.Two minor improvements for this refactored expression:
==should be===for strict comparison (typically enforced byeqeqeqlinting rule)estimateType?.typeuses unnecessary optional chaining sinceestimateType &&already guarantees it's truthy✨ Suggested improvement
const isEstimatePointValid = isEmpty(cycleDetails?.progress_snapshot || {}) - ? !!(estimateType && estimateType?.type == EEstimateSystem.POINTS) + ? !!(estimateType && estimateType.type === EEstimateSystem.POINTS) : !isEmpty(cycleDetails?.progress_snapshot?.estimate_distribution || {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/cycles/analytics-sidebar/sidebar-details.tsx` around lines 53 - 55, The conditional computing isEstimatePointValid should use strict equality and remove redundant optional chaining: update the ternary expression in isEstimatePointValid (which references cycleDetails, estimateType, EEstimateSystem, isEmpty, progress_snapshot, and estimate_distribution) so the comparison uses === instead of == and drop the extra optional chaining on estimateType.type (since estimateType is already checked truthy); keep the same logical branches but change the expression to use estimateType.type === EEstimateSystem.POINTS and access estimate_distribution via cycleDetails.progress_snapshot without the unnecessary ?. chaining.apps/web/core/components/onboarding/steps/profile/root.tsx (1)
144-144: Consider simplifying the first clause with De Morgan's law.The double negation in the first part can be flattened for improved readability while preserving the logic.
✨ Suggested simplification
- const isButtonDisabled = !(!isSubmitting && isValid) || (!isPasswordAlreadySetup && !isValidPassword); + const isButtonDisabled = isSubmitting || !isValid || (!isPasswordAlreadySetup && !isValidPassword);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/onboarding/steps/profile/root.tsx` at line 144, The condition for isButtonDisabled uses a double negation; replace the first clause !(!isSubmitting && isValid) with its De Morgan equivalent (isSubmitting || !isValid) to improve readability while keeping the second clause (!isPasswordAlreadySetup && !isValidPassword) unchanged; update the declaration of isButtonDisabled accordingly (referencing the isButtonDisabled variable and the isSubmitting, isValid, isPasswordAlreadySetup, isValidPassword symbols).apps/web/core/components/onboarding/steps/usecase/root.tsx (1)
84-84: Consider simplifying with De Morgan's law for readability.Same pattern as
role/root.tsx— the double negation can be expressed more directly.✨ Suggested simplification
- const isButtonDisabled = !(!isSubmitting && isValid); + const isButtonDisabled = isSubmitting || !isValid;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/onboarding/steps/usecase/root.tsx` at line 84, The expression for isButtonDisabled uses a double negation which reduces readability; replace the current logic in the isButtonDisabled declaration with the equivalent, clearer condition using De Morgan's law: set isButtonDisabled to true when isSubmitting is true OR isValid is false (i.e., use isSubmitting || !isValid) so the variable name directly matches the boolean expression and mirrors the pattern used in role/root.tsx.apps/web/core/components/onboarding/steps/role/root.tsx (1)
90-90: Consider simplifying with De Morgan's law for readability.The expression
!(!isSubmitting && isValid)is logically correct but the double negation is harder to parse. Using De Morgan's law, this simplifies to a more direct form.✨ Suggested simplification
- const isButtonDisabled = !(!isSubmitting && isValid); + const isButtonDisabled = isSubmitting || !isValid;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/onboarding/steps/role/root.tsx` at line 90, The isButtonDisabled assignment uses double negation; change it to a direct boolean expression: set isButtonDisabled to true when isSubmitting is true or when isValid is false (i.e., use a straightforward OR of isSubmitting and the negation of isValid), replacing the current !(!isSubmitting && isValid) usage in the component where isButtonDisabled, isSubmitting, and isValid are defined.apps/web/core/components/estimates/root.tsx (1)
140-140: Missed redundant ternaries on adjacent lines.Lines 140 and 147 contain the same
no-unneeded-ternarypattern that this PR aims to fix. SinceestimateToUpdateandestimateToDeleteare already typed asstring | undefined, the ternary expressions are redundant.♻️ Proposed fix
<UpdateEstimateModal workspaceSlug={workspaceSlug} projectId={projectId} - estimateId={estimateToUpdate ? estimateToUpdate : undefined} + estimateId={estimateToUpdate} isOpen={!!estimateToUpdate} handleClose={() => setEstimateToUpdate(undefined)} /> <DeleteEstimateModal workspaceSlug={workspaceSlug} projectId={projectId} - estimateId={estimateToDelete ? estimateToDelete : undefined} + estimateId={estimateToDelete} isOpen={!!estimateToDelete} handleClose={() => setEstimateToDelete(undefined)} />Also applies to: 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/estimates/root.tsx` at line 140, The JSX uses redundant ternaries when passing optional values: replace occurrences like estimateId={estimateToUpdate ? estimateToUpdate : undefined} and the analogous line using estimateToDelete with the direct values (estimateId={estimateToUpdate} and estimateId={estimateToDelete}) since estimateToUpdate and estimateToDelete are already typed string | undefined; update the JSX props in the root component where estimateId is set to remove the unnecessary ? : expression.apps/admin/components/instance/setup-form.tsx (1)
224-224: Drop the redundanterrorData.type &&guard.
errorData.type === EErrorCodes.INVALID_EMAILanderrorData.type === EErrorCodes.INVALID_PASSWORDalready returnfalsewhentypeis unset, so the extra left-hand check is just noise and can yieldundefinedinstead of a plain boolean.Suggested cleanup
- hasError={errorData.type && errorData.type === EErrorCodes.INVALID_EMAIL} + hasError={errorData.type === EErrorCodes.INVALID_EMAIL} @@ - hasError={errorData.type && errorData.type === EErrorCodes.INVALID_PASSWORD} + hasError={errorData.type === EErrorCodes.INVALID_PASSWORD}Also applies to: 268-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/components/instance/setup-form.tsx` at line 224, Remove the redundant left-hand guard from the hasError props: replace expressions like "errorData.type && errorData.type === EErrorCodes.INVALID_EMAIL" and the similar check for EErrorCodes.INVALID_PASSWORD with the direct comparison "errorData.type === EErrorCodes.INVALID_EMAIL" and "errorData.type === EErrorCodes.INVALID_PASSWORD" respectively (in the component where hasError is set), so the prop receives a plain boolean result; update occurrences around the hasError usage (the lines referencing EErrorCodes.INVALID_EMAIL and EErrorCodes.INVALID_PASSWORD) accordingly.apps/web/core/components/issues/issue-layouts/list/default.tsx (1)
114-114: Renameis_listtoisList.The boolean simplification is fine, but this touched local still uses snake_case instead of the repo's camelCase convention.
As per coding guidelines, "Use camelCase for variable and function names".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-layouts/list/default.tsx` at line 114, Rename the local boolean variable is_list to isList to follow the repository's camelCase convention; update the declaration (currently const is_list = group_by === null;) and all usages of is_list within the file (including JSX, functions, props, or conditional checks) to isList, and ensure any related identifiers derived from group_by remain untouched unless they also violate camelCase conventions—verify the component compiles and tests pass after replacing all references.apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx (1)
78-78: Consider further simplification.The expression
storedValue ? storedValue === true : falseis still a ternary. SinceuseLocalStorageis typed withboolean, this can be simplified tostoredValue === truewhich handles bothfalseandnull/undefinedcases correctly.♻️ Proposed simplification
- const isSidebarCollapsed = storedValue ? storedValue === true : false; + const isSidebarCollapsed = storedValue === true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx at line 78, The line computing isSidebarCollapsed can be simplified: replace the ternary expression using storedValue with a direct boolean comparison. Update the use of storedValue in the isSidebarCollapsed assignment (related to useLocalStorage and storedValue) to use storedValue === true so it handles false and null/undefined correctly and removes the unnecessary ternary.apps/web/core/components/project/dropdowns/filters/access.tsx (1)
45-45: Same type consideration as other filter components.When
appliedFiltersisnull, optional chaining returnsundefined. This follows the same pattern flagged in other files. IfFilterOptionrequires strictboolean, consider adding?? false.♻️ Proposed fix for type safety
- isChecked={appliedFilters?.includes(`${access.key}`)} + isChecked={appliedFilters?.includes(`${access.key}`) ?? false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/project/dropdowns/filters/access.tsx` at line 45, The isChecked prop uses optional chaining on appliedFilters (isChecked={appliedFilters?.includes(`${access.key}`)}) which can produce undefined when appliedFilters is null; update the check in the FilterOption render (reference: appliedFilters, access.key, FilterOption.isChecked) to return a strict boolean (e.g., coerce with ?? false or Boolean(...)) so isChecked always receives a boolean value.apps/web/core/components/modules/dropdowns/filters/target-date.tsx (1)
68-68: Consistent with other filter components - same type consideration applies.Same pattern as other filter files:
appliedFilters?.includes()returnsboolean | undefined. Consider adding?? falsefor strict type safety ifFilterOptionrequiresboolean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/modules/dropdowns/filters/target-date.tsx` at line 68, The isChecked prop currently uses appliedFilters?.includes(option.value) which can be boolean | undefined; update the value passed to isChecked to guarantee a boolean (e.g., use a nullish-coalescing fallback or Boolean cast) so it never supplies undefined; change the expression around appliedFilters?.includes(option.value) in target-date.tsx (the isChecked prop where appliedFilters, option.value, and FilterOption are involved) to return false instead of undefined for strict type safety.apps/web/core/components/inbox/inbox-filter/filters/state.tsx (1)
63-63: Optional chaining is redundant here.Since
filterValueis initialized with a fallback to[]on line 32, it's always an array and neverundefined. The?.onfilterValue?.includes()is unnecessary, though harmless.✨ Optional cleanup
- isChecked={filterValue?.includes(state?.id)} + isChecked={filterValue.includes(state.id)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/inbox/inbox-filter/filters/state.tsx` at line 63, The optional chaining on filterValue is redundant because filterValue is initialized to [] (so never undefined); update the expression in the component to call includes directly (replace filterValue?.includes(state?.id) with filterValue.includes(state?.id)), keeping the optional chaining on state?.id if state can be nullish; this change targets the use of filterValue and state?.id in the isChecked prop.apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx (1)
42-42: Consider further simplification.The expression
storedValue ? storedValue === true : falseis correct but redundant. SinceuseLocalStoragereturns a parsed boolean ornull, this can be simplified to juststoredValue === true, which handles all cases:
true === true→truefalse === true→falsenull === true→false♻️ Suggested simplification
- const isSidebarCollapsed = storedValue ? storedValue === true : false; + const isSidebarCollapsed = storedValue === true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx at line 42, The conditional expression assigning isSidebarCollapsed is redundant; replace the current ternary (storedValue ? storedValue === true : false) with a direct comparison storedValue === true to handle true/false/null consistently; update the assignment where isSidebarCollapsed is defined (using the value returned by useLocalStorage/storedValue) to use storedValue === true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e823bea0-253f-44fa-820e-7527f407dac5
📒 Files selected for processing (91)
apps/admin/app/(all)/(dashboard)/general/intercom.tsxapps/admin/app/(all)/(home)/sign-in-form.tsxapps/admin/components/instance/setup-form.tsxapps/admin/providers/user.provider.tsxapps/space/components/account/auth-forms/auth-root.tsxapps/space/components/account/auth-forms/password.tsxapps/space/components/issues/filters/labels.tsxapps/space/components/issues/filters/priority.tsxapps/space/components/issues/filters/state.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/[moduleId]/page.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsxapps/web/core/components/account/auth-forms/password.tsxapps/web/core/components/account/auth-forms/reset-password.tsxapps/web/core/components/account/auth-forms/set-password.tsxapps/web/core/components/chart/utils.tsapps/web/core/components/common/filters/created-at.tsxapps/web/core/components/common/filters/created-by.tsxapps/web/core/components/cycles/analytics-sidebar/sidebar-details.tsxapps/web/core/components/cycles/archived-cycles/header.tsxapps/web/core/components/cycles/cycles-view-header.tsxapps/web/core/components/cycles/dropdowns/estimate-type-dropdown.tsxapps/web/core/components/cycles/dropdowns/filters/end-date.tsxapps/web/core/components/cycles/dropdowns/filters/start-date.tsxapps/web/core/components/cycles/dropdowns/filters/status.tsxapps/web/core/components/dropdowns/cycle/cycle-options.tsxapps/web/core/components/estimates/root.tsxapps/web/core/components/gantt-chart/views/week-view.tsapps/web/core/components/inbox/inbox-filter/filters/date.tsxapps/web/core/components/inbox/inbox-filter/filters/labels.tsxapps/web/core/components/inbox/inbox-filter/filters/members.tsxapps/web/core/components/inbox/inbox-filter/filters/priority.tsxapps/web/core/components/inbox/inbox-filter/filters/state.tsxapps/web/core/components/inbox/inbox-filter/filters/status.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/extra-options.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/group-by.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/order-by.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/sub-group-by.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/assignee.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/created-by.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/cycle.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/due-date.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/labels.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/mentions.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/module.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/priority.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/project.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/start-date.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/state-group.tsxapps/web/core/components/issues/issue-layouts/filters/header/filters/state.tsxapps/web/core/components/issues/issue-layouts/list/default.tsxapps/web/core/components/issues/issue-layouts/quick-action-dropdowns/issue-detail.tsxapps/web/core/components/issues/issue-layouts/quick-add/root.tsxapps/web/core/components/issues/issue-layouts/utils.tsxapps/web/core/components/modules/analytics-sidebar/issue-progress.tsxapps/web/core/components/modules/analytics-sidebar/root.tsxapps/web/core/components/modules/archived-modules/header.tsxapps/web/core/components/modules/dropdowns/filters/lead.tsxapps/web/core/components/modules/dropdowns/filters/members.tsxapps/web/core/components/modules/dropdowns/filters/start-date.tsxapps/web/core/components/modules/dropdowns/filters/status.tsxapps/web/core/components/modules/dropdowns/filters/target-date.tsxapps/web/core/components/modules/modal.tsxapps/web/core/components/modules/module-view-header.tsxapps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/steps/profile/root.tsxapps/web/core/components/onboarding/steps/role/root.tsxapps/web/core/components/onboarding/steps/usecase/root.tsxapps/web/core/components/power-k/ui/pages/open-entity/project-settings-menu.tsxapps/web/core/components/power-k/ui/pages/open-entity/workspace-settings-menu.tsxapps/web/core/components/project-states/options/delete.tsxapps/web/core/components/project-states/state-item-title.tsxapps/web/core/components/project-states/state-item.tsxapps/web/core/components/project/dropdowns/filters/access.tsxapps/web/core/components/project/dropdowns/filters/created-at.tsxapps/web/core/components/project/dropdowns/filters/lead.tsxapps/web/core/components/project/dropdowns/filters/members.tsxapps/web/core/components/workspace-notifications/notification-app-sidebar-option.tsxapps/web/core/components/workspace-notifications/sidebar/filters/menu/root.tsxapps/web/core/components/workspace/sidebar/sidebar-menu-items.tsxapps/web/core/lib/wrappers/store-wrapper.tsxapps/web/core/store/notifications/workspace-notifications.store.tsapps/web/helpers/views.helper.tspackages/editor/src/core/extensions/custom-link/helpers/autolink.tspackages/editor/src/core/extensions/slash-commands/command-items-list.tsxpackages/ui/src/collapsible/collapsible.tsxpackages/ui/src/form-fields/password/indicator.tsxpackages/utils/src/rich-filters/operations/traversal/core.ts
Comment on lines
+67
to
68
| const isTelemetryEnabledParam = searchParams?.get("is_telemetry_enabled") === "True" || true; | ||
| const errorCode = searchParams?.get("error_code") || undefined; |
Contributor
There was a problem hiding this comment.
Restore three-state parsing for is_telemetry_enabled.
searchParams?.get("is_telemetry_enabled") === "True" || true is always true, so this setup flow can no longer honor is_telemetry_enabled=False from the query string. Because Lines 97-98 then copy that value into formData, the hidden field at Line 161 will always submit "True". That is a real behavior change in a telemetry opt-out path, and it diverges from apps/admin/app/(all)/(dashboard)/general/form.tsx:45-68, where telemetry is treated as a real boolean.
Proposed fix
- const isTelemetryEnabledParam = searchParams?.get("is_telemetry_enabled") === "True" || true;
+ const isTelemetryEnabledParam = searchParams?.get("is_telemetry_enabled");
@@
- if (isTelemetryEnabledParam) setFormData((prev) => ({ ...prev, is_telemetry_enabled: isTelemetryEnabledParam }));
+ if (isTelemetryEnabledParam !== null) {
+ setFormData((prev) => ({
+ ...prev,
+ is_telemetry_enabled: isTelemetryEnabledParam === "True",
+ }));
+ }Also applies to: 92-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/components/instance/setup-form.tsx` around lines 67 - 68, The
value for isTelemetryEnabledParam is forced true by the `... === "True" || true`
expression, so restore three-state parsing by reading
`searchParams.get("is_telemetry_enabled")` and mapping `"True" -> true`,
`"False" -> false`, and anything else -> undefined (or null) before copying into
`formData`; update the variable `isTelemetryEnabledParam` in setup-form.tsx and
any subsequent code that copies it into `formData` (and the analogous block
around the later copy) so the hidden telemetry field will submit the actual
parsed boolean or no value when unset.
apps/web/core/components/issues/issue-layouts/filters/header/filters/assignee.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/propel/src/button/helper.tsx`:
- Around line 20-22: The change updates the CSS class strings for the "danger"
and "secondary" button variants (the variant entries labeled danger and
secondary in the button helper) which alters visual tokens
(borders/shadows/background) and therefore is not lint-only; revert these two
class string edits back to their original classes to keep this PR purely lint
changes, or extract the visual adjustments into a separate styling PR (with
screenshots/design review) that modifies the danger and secondary variant
entries in the button helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fabda4b-7508-44fe-8e9b-b1a708dcc754
📒 Files selected for processing (18)
apps/space/components/issues/filters/labels.tsxapps/space/components/issues/filters/priority.tsxapps/space/components/issues/filters/state.tsxapps/space/store/cycle.store.tsapps/space/store/helpers/base-issues.store.tsapps/space/store/instance.store.tsapps/space/store/issue-detail.store.tsapps/space/store/issue-filters.store.tsapps/space/store/issue.store.tsapps/space/store/label.store.tsapps/space/store/members.store.tsapps/space/store/module.store.tsapps/space/store/profile.store.tsapps/space/store/publish/publish.store.tsapps/space/store/publish/publish_list.store.tsapps/space/store/state.store.tsapps/space/store/user.store.tspackages/propel/src/button/helper.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/space/components/issues/filters/labels.tsx
- apps/space/components/issues/filters/state.tsx
- apps/space/components/issues/filters/priority.tsx
Comment on lines
+20
to
+22
| "border border-danger-strong bg-layer-2 text-danger-secondary hover:bg-danger-subtle active:bg-danger-subtle-hover disabled:border-subtle-1 disabled:bg-layer-2 disabled:text-disabled", | ||
| secondary: | ||
| "bg-layer-2 hover:bg-layer-2-hover active:bg-layer-2-active disabled:bg-layer-transparent text-secondary disabled:text-disabled border border-strong disabled:border-subtle-1 shadow-raised-100", | ||
| "border border-strong bg-layer-2 text-secondary shadow-raised-100 hover:bg-layer-2-hover active:bg-layer-2-active disabled:border-subtle-1 disabled:bg-layer-transparent disabled:text-disabled", |
Contributor
There was a problem hiding this comment.
Please keep this PR lint-only or split the button restyling out.
These class changes modify the rendered appearance of two shared variants by introducing new base border/shadow/background tokens. That conflicts with the stated goal of a mechanical oxlint cleanup with no behavioral changes, and it adds UI regression risk to what should be an easy-to-validate lint PR. Please revert these visual tweaks here or move them into a separate styling PR with screenshots/design review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/propel/src/button/helper.tsx` around lines 20 - 22, The change
updates the CSS class strings for the "danger" and "secondary" button variants
(the variant entries labeled danger and secondary in the button helper) which
alters visual tokens (borders/shadows/background) and therefore is not
lint-only; revert these two class string edits back to their original classes to
keep this PR purely lint changes, or extract the visual adjustments into a
separate styling PR (with screenshots/design review) that modifies the danger
and secondary variant entries in the button helper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolve oxlint warnings for two rules:
x ? true : false→x,x ? false : true→!xObject.assign()91 files changed, all mechanical transformations with no behavioral changes.
Context
These warnings were introduced by the addition of
oxlintto the project's lint toolchain. The pre-commit hook (oxlint --deny-warnings) currently fails on existing code.This is 1 of 4 PRs splitting the full lint cleanup into reviewable chunks.
Test plan
pnpm exec oxlint --deny-warningsshows 0no-unneeded-ternary/no-map-spreadwarningsSummary by CodeRabbit
Bug Fixes
Refactor