Skip to content

fix(component): compare field values in BaseComponent.__eq__#6493

Merged
masenf merged 2 commits into
reflex-dev:mainfrom
FarhanAliRaza:component-eq
May 12, 2026
Merged

fix(component): compare field values in BaseComponent.__eq__#6493
masenf merged 2 commits into
reflex-dev:mainfrom
FarhanAliRaza:component-eq

Conversation

@FarhanAliRaza
Copy link
Copy Markdown
Contributor

bool() on a generator is always True, so any two same-class components compared equal. Use all() to actually evaluate the per-field comparisons.

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

bool() on a generator is always True, so any two same-class components
compared equal. Use all() to actually evaluate the per-field comparisons.
@FarhanAliRaza FarhanAliRaza requested a review from a team as a code owner May 12, 2026 12:28
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 12, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing FarhanAliRaza:component-eq (43a0ff6) with main (2c9f76c)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 12, 2026

Greptile Summary

This PR fixes a genuine bug in BaseComponent.__eq__ where bool(generator_expression) was used — a generator object is always truthy, so two components of the same class always compared equal regardless of their field values. Changing to all(generator_expression) correctly iterates and short-circuits on the first unequal field.

  • Core fix (boolall): correctly evaluates per-field comparisons for plain Python-typed fields such as str, int, and nested component children.
  • Potential runtime exception: Var.__eq__ returns a BooleanVar rather than a Python bool, and BooleanVar.__bool__ raises VarTypeError. Fields like ref: Var | None, special_props: list[Var], and event_triggers: dict[str, EventChain | Var] can hold Var instances; comparing two components that have any such field populated will now raise instead of silently returning True.
  • New test in test_component.py covers the fixed case well but does not exercise Var-valued fields.

Confidence Score: 3/5

Safe to merge only for components whose fields hold plain Python values; components with any Var-valued field will now raise VarTypeError on equality comparison instead of silently returning True.

The fix correctly addresses the generator-truthiness bug and the new test demonstrates the intended behaviour. However, Var.eq returns a BooleanVar whose bool unconditionally raises VarTypeError, so any equality check involving a component that carries a reactive Var prop (ref, special_props, user-supplied state vars) will now throw at runtime.

packages/reflex-base/src/reflex_base/components/component.py — the eq body needs a guard (e.g. identity fallback or isinstance(result, Var) check) before all() evaluates truthiness on field comparisons.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/components/component.py Fixes bool(generator) to all(generator) in BaseComponent.__eq__, but the generator can yield BooleanVar (not bool) for Var-valued fields, causing VarTypeError at runtime.
tests/units/components/test_component.py Adds test_component_equality_compares_fields covering same-class/different-class and nested children cases; does not exercise fields containing Var values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["BaseComponent.__eq__(self, value)"] --> B{"type(self) is type(value)?"}
    B -- No --> C["return False"]
    B -- Yes --> D["all(getattr(self,k) == getattr(value,k) for k in get_fields())"]
    D --> E{"field value is plain Python type?"}
    E -- Yes --> F["== returns bool, all() evaluates correctly"]
    F --> G["return True or False"]
    E -- No --> H["field value is Var, Var.__eq__ returns BooleanVar"]
    H --> I["all() calls bool(BooleanVar)"]
    I --> J["BooleanVar.__bool__ raises VarTypeError"]
Loading

Reviews (1): Last reviewed commit: "fix(component): compare field values in ..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/components/component.py
Var equality returns a BooleanVar and bool-ifying it raises VarTypeError,
so the naive all(==) walk blew up on components with Var props or
list/dict fields containing Vars. Compare Vars structurally via
Var.equals and walk containers element-wise.
@masenf masenf merged commit cb9ae99 into reflex-dev:main May 12, 2026
69 checks passed
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.

2 participants