Skip to content

do not compile stateful components in dev mode#6199

Merged
adhami3310 merged 2 commits intomainfrom
exp-isinstance-check
Mar 20, 2026
Merged

do not compile stateful components in dev mode#6199
adhami3310 merged 2 commits intomainfrom
exp-isinstance-check

Conversation

@adhami3310
Copy link
Copy Markdown
Member

No description provided.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 19, 2026

Merging this PR will not alter performance

✅ 8 untouched benchmarks


Comparing exp-isinstance-check (5cb1bea) with main (6767870)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR is a small experimental refactor in reflex/compiler/compiler.py that introduces a TypeGuard-annotated helper function is_stateful_component to replace a raw isinstance(component, StatefulComponent) check in _get_shared_components_recursive. The goal is to improve static type-checker narrowing at the call-site.

Key observations:

  • Semantic difference vs isinstance: BaseComponent is backed by BaseComponentMeta(FieldBasedMeta, ABCMeta), meaning isinstance goes through ABCMeta.__instancecheck__ and respects virtual subclass registration and __subclasshook__. The new StatefulComponent in type(component).__mro__ check bypasses both. No virtual subclasses are registered today, but the trade-off is undocumented.
  • Missing docstring: is_stateful_component is the only function in the file without a docstring, breaking the module's consistent documentation pattern.
  • Simpler alternative: The same TypeGuard benefit could be achieved by wrapping isinstance directly — return isinstance(component, StatefulComponent) — while retaining full ABC compatibility.
  • The experimental nature of the PR (noted in the title) suggests this is not yet intended for production merge.

Confidence Score: 3/5

  • Functionally safe today but introduces a subtle semantic divergence from isinstance that is undocumented and could silently break if ABC virtual-subclass registration is ever used.
  • The change is small and produces correct results given the current codebase (no virtual subclasses of StatefulComponent exist). However, replacing isinstance with an MRO lookup against an ABCMeta-backed class is a semantically different operation, and the function lacks a docstring. The PR also carries an "experiment" prefix, indicating it is not intended as final code. These points together prevent a higher confidence rating.
  • reflex/compiler/compiler.py — specifically the new is_stateful_component function and its interaction with BaseComponentMeta(ABCMeta).

Important Files Changed

Filename Overview
reflex/compiler/compiler.py Introduces is_stateful_component TypeGuard helper that replaces an isinstance check with a direct MRO lookup; the helper lacks a docstring and the MRO approach diverges semantically from isinstance because BaseComponent uses an ABCMeta-derived metaclass.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_get_shared_components_recursive(component)"] --> B{"is_stateful_component(component)\n(MRO check)"}
    B -- True --> C{"component.references > 1\nand is_prod_mode()"}
    B -- False --> G["Skip — recurse into children only"]
    C -- True --> D["Reset rendered_as_shared = False"]
    D --> E["Collect dynamic imports, custom code, imports"]
    E --> F["Set rendered_as_shared = True"]
    C -- False --> G

    subgraph "is_stateful_component implementation"
        H["StatefulComponent in type(component).__mro__"]
        I["isinstance(component, StatefulComponent)\n(original, uses ABCMeta.__instancecheck__)"]
        H -. "bypasses ABC virtual-subclass registration" .-> I
    end

    style H fill:#ffe0b2
    style I fill:#c8e6c9
Loading

Last reviewed commit: "exp isinstance check"

@adhami3310 adhami3310 changed the title experiment: isinstance check do not compile stateful components in dev mode Mar 19, 2026
@adhami3310 adhami3310 merged commit 7ee3026 into main Mar 20, 2026
47 checks passed
@adhami3310 adhami3310 deleted the exp-isinstance-check branch March 20, 2026 04:26
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