ENG-9892: feat(plugins): add register_route hook and get_plugin lookup#6728
Conversation
Plugins that contribute pages (e.g. auth) previously needed global state and circular-import workarounds to find their configured instance and inject routes. Instead: - Plugin.register_route(app) runs once per app at the start of compile_app, letting plugins add pages before routes are resolved. - get_plugin(cls) resolves the configured plugin instance from the rxconfig plugins list, raising ConfigError on ambiguous matches. - add_page invalidates the cached router so late-added routes resolve.
Greptile SummaryThis PR adds plugin-owned route registration and lookup support. The main changes are:
Confidence Score: 4/5This is close, but the router cache issue should be fixed before merging.
reflex/compiler/compiler.py Important Files Changed
Reviews (3): Last reviewed commit: "feat(plugins): stage register_route hook..." | Re-trigger Greptile |
Merging this PR will not alter performance
Comparing Footnotes
|
tests/units/reflex_base/plugins/test_base.py collided with tests/units/vars/test_base.py during pytest collection without an __init__.py, and towncrier required feature fragments for PR reflex-dev#6728 in both the root package and reflex-base.
Rework the register_route hook so plugins contribute pages through a staged add_page capability instead of mutating the app directly. All plugin contributions are validated and collected first, then committed together, so a failing hook cannot leave partial pages or dynamic route variables behind. Split App.add_page into a pure _prepare_page (validation/normalization, the extension point for App subclasses) and a fixed non-fallible _commit_page, and give the hook context add_page, has_app_page, and app_type instead of the mutable App. Detect app/plugin route conflicts, structural dynamic-segment mismatches, and inconsistent dynamic argument types up front, and invalidate the cached router when a page is added after it was first built.
| app._apply_decorated_pages() | ||
| config = get_config() | ||
| _register_plugin_routes(app, config.plugins) | ||
| app._pages = {} |
There was a problem hiding this comment.
This reset still bypasses the router-cache invalidation added to _commit_page(). When a long-lived app has already served a request, app.router can be cached from the old route set. A later compile replaces _pages here and rebuilds it below, but the cached router keeps matching the old route list because this direct assignment never clears app.__dict__["router"]. Requests after that compile can continue resolving against stale routes until another page commit happens to invalidate the cache.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Plugins that contribute pages (e.g. auth) previously needed global state and circular-import workarounds to find their configured instance and inject routes. Instead:
All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features: