Repository navigation
Ssr plugin wip - #6800
Ssr plugin wip#6800benedikt-bartscher wants to merge 27 commits into
Conversation
Merging this PR will not alter performance1 warning
Comparing 
|
 [Medium impact] The changes affect page rendering, caching, and state loading for apps that opt in. The PR should not merge until cache clearing survives in-flight renders and requires a configured credential. Findings
SummaryAdds experimental SSR data loading and an ISR page server with shared caching and on-demand refresh.
Reviews (10) · Last reviewed commit: "Cache ISR pages by raw path and drop the..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| revalidation endpoint requires a matching ``X-Reflex-ISR-Token`` header when | ||
| the ``REFLEX_ISR_REVALIDATE_TOKEN`` env var is set. | ||
|
|
||
| Run in production as an app factory, e.g. | ||
| ``granian --factory reflex.isr:create_isr_app``. | ||
|
|
||
| Args: |
There was a problem hiding this comment.
pipe.sadd(self._TAG_PREFIX + tag, key) writes tag index entries with no TTL. When a page's TTL expires naturally in Redis, the corresponding entry in the tag set remains indefinitely. On long-lived deployments with many unique tags or frequent cache churn, these orphaned set members grow without bound. When invalidate_tag is later called, it deletes pages that no longer exist (harmless but wasteful), and the sets themselves are never reaped unless explicitly invalidated.
Consider setting a TTL on each tag key (e.g. EXPIRE tag_key ttl) in the pipeline, or using EXPIREAT set to the furthest expiry of any member.
| result = handler.fn(substate) | ||
| if asyncio.iscoroutine(result): | ||
| result = await result | ||
| if inspect.isgenerator(result): | ||
| for _ in result: | ||
| pass | ||
| elif inspect.isasyncgen(result): | ||
| async for _ in result: |
There was a problem hiding this comment.
Generator and async-generator handlers are iterated with for _ in result, which correctly preserves self mutations but silently drops every yielded value. Reflex generator handlers can yield EventSpec objects (e.g. yield SomeState.other_handler()) or explicit state deltas that operate on other substates. Those cross-substate side effects will not be reflected in the SSR output. Apps that use chained generator events for data loading may render with partially hydrated state without any warning in the logs.
Comments Outside DiffThese findings could not be posted inline.
|
Upstream replaced the per-substate reducer providers in context.js with a useSyncExternalStore state store. The SSR state seeding is ported onto it: ClientStateProvider builds its store from the SSR state with the compiled defaults as fallback, and ServerStateProvider reads SSR substates through a store whose getSnapshot falls back to initialState. With SSR enabled, initialStateHashes is emitted empty, since the store may not start from the compiled defaults and the first hydrate must then return the full state. pyi_hashes.json was regenerated.
| await self.cache.set( | ||
| key, | ||
| CachedPage( | ||
| html=result.html, | ||
| generated_at=time.time(), | ||
| revalidate=revalidate, | ||
| tags=result.tags, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
revalidate_path can be undone by a render already in progress. If that render reads old content, then an update deletes the cache entry, _render_and_store still writes the old HTML back afterward. The next request gets the invalidated content again; with revalidate=0, it stays cached indefinitely. Track an invalidation version and reject writes from renders that started before it changed.
| if revalidate_token and ( | ||
| request.headers.get("x-reflex-isr-token") != revalidate_token | ||
| ): | ||
| return JSONResponse({"error": "unauthorized"}, status_code=401) |
There was a problem hiding this comment.
If the page server is publicly reachable and REFLEX_ISR_REVALIDATE_TOKEN is unset or empty, anyone can POST a path or tag to /_isr/revalidate and delete cached pages. Repeated invalidation and GET requests defeat the cache and force render work across workers. Disable this endpoint without a configured token, or reject requests until one is configured.
How this was verified: The public POST route skips its only credential check when the token is empty and passes the caller's path or tag directly to cache deletion.
Pass on_load payloads and query parameters to SSR load handlers, capture every remaining segment for catch-all routes, and release an ISR lock only while its acquirer still owns it. Move the SSR compile tests to module level.
Move the inline imports in the ISR server and the SSR modules to module level (only reflex.app stays type-only to avoid a cycle), add docstrings to the ISR handlers and test helpers, and add feature fragments for the SSR and ISR support.
Install express into .web only for an SSR build, since isr-render.mjs is the only code that imports it.
extract_route_params now URL-decodes each value and splits only the catch-all on slashes, so server-rendered params match what the client sees.
Starlette decodes request.url.path, so /blog/a%2Fb was rendered and cached as /blog/a/b; the page server now uses the raw path, and the docs state that pages are cached per path without the query string. SsrMode.BOT_ONLY was never implemented and behaved like ALWAYS.
#6173