feat: add app_wraps to VarData for Var-driven provider injection#6447
feat: add app_wraps to VarData for Var-driven provider injection#6447FarhanAliRaza wants to merge 2 commits intoreflex-dev:mainfrom
Conversation
Lets Vars declare app-level wrapper components in their VarData so the compiler can mount providers (state context, event loop, upload, etc.) based on what's actually used, instead of relying on hardcoded chains or special-case logic. State/event-loop providers now ride along on VarData.from_state and the events-hook helper, and UploadFilesProvider is mounted when selected_files/upload_file is referenced — even without an Upload component on the page. Layout renders the assembled chain so AppWrap reduces to hooks + children.
Merging this PR will degrade performance by 20.3%
Performance Changes
Comparing 
Footnotes
|
Greptile SummaryThis PR replaces the hardcoded All P2 findings — a missing type annotation on an internal helper and a tag-only equality semantics note — are non-blocking. Confidence Score: 4/5Safe to merge; all findings are P2 style/latent concerns with no current-code defects. Only P2 issues found: an
Important Files Changed
Sequence DiagramsequenceDiagram
participant V as Var/VarData
participant C as Component
participant CP as DefaultCollectorPlugin
participant MP as MemoizeStatefulPlugin
participant R as _resolve_app_wrap_components
participant T as app_root_template
V->>V: VarData(app_wraps=[(priority, Provider)])
C->>CP: _get_vars() / _get_hooks_internal()
CP->>CP: collect_var_app_wraps_for_component()
CP->>CP: _ingest_var_data_app_wraps() → page_app_wrap_components
MP->>CP: collect_var_app_wraps_in_subtree() (snapshot boundary)
CP->>R: page_app_wrap_components
R->>R: merge Component._get_app_wrap_components()
R->>R: fixpoint: collect app_wraps from wrapper subtrees
R->>T: ordered wrap chain (priority desc)
T->>T: Layout renders StateProvider > EventLoopProvider > ... > AppWrap(hooks + children)
Reviews (1): Last reviewed commit: "feat: add app_wraps to VarData for Var-d..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _ingest_var_data_app_wraps( | ||
| wraps_by_key: dict[tuple[int, str], Component], |
There was a problem hiding this comment.
var_data parameter reduces type safety
var_data is annotated as Any but the function immediately accesses .app_wraps and .hooks. A non-VarData argument silently passes type-checking and will raise AttributeError at runtime. All call sites pass VarData, so the annotation should match.
| @@ -267,6 +287,7 @@ def merge(*all: VarData | None) -> VarData | None: | |||
| deps=deps, | |||
| position=position, | |||
| components=components, | |||
| app_wraps=tuple(app_wraps_list), | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
app_wraps equality keyed only on (priority, tag) — different configs silently deduplicate
_identity_key compares app_wraps entries by (priority, component.tag or type(component).__name__), so two providers sharing the same tag and priority are considered equal even when their props differ (e.g., a pre-configured UploadFilesProvider with a custom max_file_size would be silently dropped if a plain one already exists at that priority). The design is intentional for the current stateless providers, but the asymmetry with components — which uses id(component) for strict identity — means future wrappers with meaningful props could be silently misconfigured without any error.
| "postcss-import": {}, | ||
| autoprefixer: {}, | ||
| }, | ||
| }; | ||
| """, | ||
| }; | ||
| """, | ||
| ) |
There was a problem hiding this comment.
}; and the closing """ delimiter now carry a leading tab, producing inconsistent indentation in the written postcss.config.js stub and trailing whitespace after the closing brace. Postcss is whitespace-agnostic so tests still pass, but it differs from the rest of the file's style.
| "postcss-import": {}, | |
| autoprefixer: {}, | |
| }, | |
| }; | |
| """, | |
| }; | |
| """, | |
| ) | |
| "postcss-import": {}, | |
| autoprefixer: {}, | |
| }, | |
| }; | |
| """, | |
| ) |
EventLoopProvider now populates a module-level addEvents in $/utils/context, so JSX literals constructed outside the React-tree hoist path (e.g. ErrorBoundary.onError) can dispatch events without useContext(EventLoopContext) being lexically in scope. State and event-loop providers ride along on event-invocation VarData.app_wraps (and via _get_event_app_wraps) so they still mount in the app root. connectErrors stays on useContext since it drives re-renders; AppWrap is now a Fragment rendering the chain in its body.
1 participant
Lets Vars declare app-level wrapper components in their VarData so the compiler can mount providers (state context, event loop, upload, etc.) based on what's actually used, instead of relying on hardcoded chains or special-case logic. State/event-loop providers now ride along on VarData.from_state and the events-hook helper, and UploadFilesProvider is mounted when selected_files/upload_file is referenced — even without an Upload component on the page. Layout renders the assembled chain so AppWrap reduces to hooks + children.
All Submissions:
Type of change
Please delete options that are not relevant.
New feature (non-breaking change which adds functionality)
This change requires a documentation update
New Feature Submission:
Changes To Core Features: