Skip to content

Conversation

@gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Oct 29, 2025

Description Of Changes

This change was prompted by a near-miss bug in #6872

Frankly, I'm surprised this hasn't been a linting rule of ours all along.

Enforced strict equality checks (=== and !==) throughout the Admin UI codebase by adding the eqeqeq ESLint rule. This prevents potential bugs from JavaScript's type coercion behavior and makes null/undefined checks more explicit.

Code Changes

  • Added eqeqeq: ["error", "always"] ESLint rule to enforce strict equality
  • Updated ConnectedCircle.tsx to explicitly check for null and undefined
  • Updated helpers.ts type guards to use strict equality for null checks
  • Updated useTableState.ts filter cleanup to explicitly check for null and undefined
  • Updated DataUsesForm.tsx skip condition to use strict equality
  • Updated DatasetYamlForm.tsx type guard to use strict equality
  • Updated taxonomy/helpers.ts to use strict equality and simplified label fallback logic

Steps to Confirm

  1. Run npm run lint in the clients/admin-ui directory to verify no ESLint errors
  2. Test ConnectedCircle component with undefined, null, true, and false values to ensure correct colors are displayed
  3. Verify table filtering functionality works correctly in any table view
  4. Test data uses form with and without vendor selection
  5. Test dataset YAML import functionality
  6. Verify taxonomy entity displays work correctly

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@gilluminate gilluminate requested a review from a team as a code owner October 29, 2025 19:03
@gilluminate gilluminate requested review from eastandwestwind and removed request for a team October 29, 2025 19:03
@vercel
Copy link

vercel bot commented Oct 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Oct 29, 2025 7:55pm
fides-privacy-center Ignored Ignored Oct 29, 2025 7:55pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Enforces strict equality checks (=== and !==) throughout the Admin UI codebase by adding the eqeqeq ESLint rule.

  • Added eqeqeq: ["error", "always"] to ESLint configuration to prevent type coercion bugs
  • Updated 6 files to replace loose equality (==, !=) with strict equality (===, !==)
  • All changes correctly handle null/undefined checks with explicit comparisons
  • Simplified label fallback logic in taxonomy/helpers.ts using truthy operator while maintaining equivalent behavior

The changes are straightforward refactoring to comply with the new linting rule and improve code safety.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • All changes are mechanical refactoring to enforce strict equality, with no logic changes. The transformations correctly preserve existing behavior while adding type safety.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/.eslintrc.cjs 5/5 Added eqeqeq rule to enforce strict equality checks throughout codebase
clients/admin-ui/src/features/common/ConnectedCircle.tsx 5/5 Updated nullish check to use strict equality for connected prop
clients/admin-ui/src/features/common/helpers.ts 5/5 Updated type guard functions to use strict equality for null checks
clients/admin-ui/src/features/taxonomy/helpers.ts 5/5 Replaced loose equality with strict checks and simplified label fallback logic using truthy operator

7 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this rule. This is bound to prevent some bugs. Tested connected circle and request manager table filters, everything works. Approved.
You might want to add a DX changelog entry?

@gilluminate
Copy link
Contributor Author

gilluminate commented Oct 29, 2025

You might want to add a DX changelog entry?

@lucanovera I didn't think this warranted an entry. What do you think?

@lucanovera
Copy link
Contributor

You might want to add a DX changelog entry?

@lucanovera I didn't think this warranted an entry. What do you think?

I think so. It is a DX improvement to me.

@gilluminate gilluminate force-pushed the gill/add-strict-equal-rule branch from a1a73fd to 9f44f04 Compare October 29, 2025 19:55
@gilluminate gilluminate enabled auto-merge October 29, 2025 19:57
@gilluminate gilluminate added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit 22c2cd9 Oct 29, 2025
46 checks passed
@gilluminate gilluminate deleted the gill/add-strict-equal-rule branch October 29, 2025 20:37
adamsachs pushed a commit that referenced this pull request Nov 3, 2025
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.

3 participants