Skip to content

Move more globals to RegistrationContext#6382

Open
masenf wants to merge 9 commits intomainfrom
masenf/registered-config
Open

Move more globals to RegistrationContext#6382
masenf wants to merge 9 commits intomainfrom
masenf/registered-config

Conversation

@masenf
Copy link
Copy Markdown
Collaborator

@masenf masenf commented Apr 24, 2026

contextual management of App, Config, decorated pages, and memo components

contextual management of Config, decorated pages, and memo components
@masenf masenf requested a review from a team as a code owner April 24, 2026 11:58
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 24, 2026

Merging this PR will not alter performance

✅ 9 untouched benchmarks


Comparing masenf/registered-config (70648df) with main (7a7a4d3)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR migrates module-level global registries (CUSTOM_COMPONENTS, EXPERIMENTAL_MEMOS, DECORATED_PAGES, bundled_libraries, and App/Config references) into RegistrationContext, enabling fully independent per-context state for parallel test runs and multi-app scenarios. The key additions are: a fork() method for shallow-copying contexts without inheriting the App/Config slots, a _set_app() guard that prevents multiple apps in one context, and new get_config() / reload_config() helpers that cache config per context.

Confidence Score: 5/5

Safe to merge — well-tested refactoring with no identified runtime regressions.

No P0 or P1 issues found. The only comment is a P2 style suggestion about using object.__setattr__ directly in prerequisites.py rather than a dedicated reset method. The global→context migration is consistent across all call sites, the lock protecting sys.path manipulation is preserved, fork() correctly isolates mutable collections, and the new test suite in test_registry.py provides solid coverage of the new behaviours.

reflex/utils/prerequisites.py — get_app(reload=True) bypasses the _set_app guard and skips clearing custom_components/memo_definitions.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/registry.py Core change: adds _config, decorated_pages, custom_components, memo_definitions, bundled_libraries, and _app fields to RegistrationContext; introduces fork(), _set_app(), and _set_config(). Well-guarded with helpful error messages.
packages/reflex-base/src/reflex_base/config.py Replaces get_config(reload=) with separate get_config() / reload_config() helpers that cache per-RegistrationContext; the _load_config_lock is preserved correctly and wraps the full sys.path + sys.modules manipulation.
packages/reflex-base/src/reflex_base/context/base.py Adds eq=False to BaseContext for identity-based dict-key semantics in _attached_context_token; fixes a latent KeyError in __exit__ by using pop(self, None).
reflex/app.py Drops DECORATED_PAGES/CUSTOM_COMPONENTS/EXPERIMENTAL_MEMOS imports; calls _registration_context._set_app(self) and reload_config() on init; uses context-scoped registries for page/memo compilation.
reflex/state.py Removes OnLoadInternalState._app_ref class var and its stale-reference reset; on_load_internal now reads the app live from RegistrationContext.get().app.
reflex/testing.py Switches from deepcopy(RegistrationContext) to .fork() for per-harness isolation; drops manual CUSTOM_COMPONENTS/EXPERIMENTAL_MEMOS clearing since they now live on the fresh forked context.
reflex/utils/prerequisites.py get_app(reload=True) clears decorated_pages and resets _app, but does not clear custom_components or memo_definitions. Stale registrations for removed components survive reload — this matches pre-existing global behavior, but could silently include removed components in compilation.
reflex/istate/shared.py Replaces get_app() prerequisite with RegistrationContext.get().app; adds a defensive event_namespace is None early-return before iterating tokens.
tests/units/conftest.py Adds _isolate_app_in_context autouse fixture using object.__setattr__ to reset _app around each test; removes the now-unnecessary preserve_memo_registries fixture.
tests/units/reflex_base/test_registry.py Comprehensive new tests covering config caching, fork isolation, app registration guards, decorated_pages isolation, custom component / memo isolation, and bundled library isolation.

Sequence Diagram

sequenceDiagram
    participant T as Test / CLI
    participant RC as RegistrationContext
    participant CF as config._load_config()
    participant AM as App Module
    participant AP as App.__init__

    T->>RC: RegistrationContext() or fork()
    RC-->>T: new context (no _app, no _config)
    T->>RC: __enter__ / set()
    T->>CF: reload_config()
    CF->>CF: acquire _load_config_lock
    CF->>CF: sys.modules.pop(rxconfig)
    CF->>CF: importlib.import_module(rxconfig)
    CF->>RC: ctx._set_config(config)
    T->>AM: importlib.reload(app_module)
    AM->>AP: rx.App()
    AP->>RC: _registration_context._set_app(self)
    Note over RC: Guards against second App in same context
    AP->>RC: reload_config()
    T->>RC: __exit__ / reset(token)
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
claude and others added 4 commits April 24, 2026 19:51
Addresses Greptile P2 review comment on #6382. The previous get_config
guarded sys.path clear/restore with a module-level RLock; this restores
the same protection around _load_config so concurrent calls (e.g. from
multiple AppHarness instances or reload_config on background threads)
cannot leave sys.path in a corrupt state.
@masenf
Copy link
Copy Markdown
Collaborator Author

masenf commented Apr 27, 2026

@greptile-apps re-review

masenf added 2 commits April 27, 2026 15:30
Accessing these properties raises a ReflexRuntimeError if the value was None at
the time it is accessed. This cleans up LBYL None checking wherever these value
are accessed.

`RegistrationContext.fork` now does NOT copy the Config, allowing both Config
and App to be loaded/reloaded in a forked registration context.
Otherwise the second load causes the RegistrationContext to raise an exception,
because _app is already set from the initial load.
@masenf
Copy link
Copy Markdown
Collaborator Author

masenf commented Apr 28, 2026

@greptile-apps re-review

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