add orjson - #6116
Conversation
Merging this PR will not alter performance
Performance Changes
Comparing Footnotes
|
Greptile SummaryThis PR adds optional
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| reflex/app.py | Adds a per-server Socket.IO codec and keeps inbound event decoding on stdlib JSON. |
| packages/reflex-components-core/src/reflex_components_core/core/_upload.py | Keeps upload event args on stdlib JSON and uses the socket JSON helper for streamed updates. |
| packages/reflex-base/src/reflex_base/utils/serializers.py | Updates serializer registration behavior and parses Plotly JSON with the engine-aware helper. |
| packages/reflex-base/src/reflex_base/utils/format.py | Adds the shared orjson helpers and socket-specific handling for unsupported JSON values. |
| packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/json.js | Adds frontend parsing for non-finite float sentinels and bare backend tokens. |
Reviews (33): Last reviewed commit: "Merge branch 'main' into try-orjson" | Re-trigger Greptile
|
the issue with orjson is that we lose NaN handling which is relevant for numpy, which is why we were using JSON5 as well |
|
Yeah, that's why i did not update it in all places |
…llback Happy path uses native JSON.parse; the catch rewrites Python"s bare Infinity/-Infinity/NaN tokens outside string literals before retrying, so only payloads that actually contain specials pay the extra cost. NaN has no JSON literal and becomes null (matches JSON.stringify).
…o null Swap bare NaN for a sentinel string before JSON.parse and revive it back to a real NaN so Python-side float(nan) round-trips to the frontend.
|
related #6339 |
Apply orjson_dumps/orjson_loads helpers to the new reflex_base package locations after upstream's restructuring.
bed6d7b to
7555e7b
Compare
There was a problem hiding this comment.
All reported issues were addressed across 35 files
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
…ex-dev#6854) * Upgrade react-router to 8.3.0 Moves the frontend from react-router 7.18.2 to 8.3.0 and adopts the new baseline it requires: - Node `MIN_VERSION` 22.12.0 -> 22.22.0 (`engines.node` of react-router 8) - Drop `react-router-dom`, removed upstream in v8. Nothing in the framework imported it; the existing stale-package pruning in `_install_frontend_packages` removes it from projects on next install. The React 19.2.7+ peer floor and Vite 7+ floor are already met by the current 19.2.8 and 8.0.16 pins, so neither moves here. Vite stays at 8.0.16 per the existing memo-rerender note. No other source changes were needed. Reflex sets no `future.v8_*` flags, so the behaviors those flags now enable by default (middleware, pass-through requests, trailing-slash-aware data requests, the Vite Environment API build path, and `splitRouteModules`) apply without config changes. Generated route modules export only a default component -- no `meta`, `loader`, or `hasErrorBoundary` -- so the `meta` `data`/`loaderData` rename, the `RouterContextProvider` context change, and the `hasErrorBoundary` removal are all inapplicable. `future.unstable_optimizeDeps` remains a valid v8 config key. Verified by building and running a multi-page app (state, websocket event round trip, `Link` client-side navigation, dynamic route, 404 fallback) in dev and prod modes, plus the embed entry's `createMemoryRouter`/`RouterProvider` path in a host page. Also confirmed the `patchReactRouterHmrRuntime` vite plugin (reflex-dev#6774) still applies: its regex matches 8.3.0's hmr runtime, the plugin's "hmr runtime changed" skip warning never fires, and the wedge it fixes does not recur -- editing an unloaded route then a loaded one still hot updates. That patch fails open, so a silent regex miss would have quietly reintroduced the bug. Adds tests pinning the upstream Node/React/Vite floors, which otherwise live only in react-router's manifest, so a future partial bump fails loudly instead of at install time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZJDfyvTWhhJ25Lpch4VmS * Correct the vite version claim in the reflex-base 0.9.8 changelog The 0.9.8 dependency-bump entry lists `vite: 8.0.16 -> 8.2.0`, but that bump was reverted before reflex-dev#6678 merged -- vite 8.2.0 reintroduces a memoized-component re-render regression, which is why the pin still carries a comment holding it at 8.0.16. The news fragment was not updated alongside the revert, so the released notes claim an upgrade that never shipped. Drops that line, and drops the "(also satisfying `vite` 8.2.0's `^8.5.23`)" aside from the same entry's postcss-override rationale: the shipped vite is 8.0.16, which requires only `postcss@^8.5.15`. The override's actual reason -- keeping transitive resolutions on a patched release (>= 8.5.18) for the security advisory -- is unchanged and retained. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZJDfyvTWhhJ25Lpch4VmS * Drop the framework-owned postcss package.json override `postcss` is already pinned directly in `DEV_DEPENDENCIES` (8.5.23), and a top-level pin satisfies and dedupes every transitive requirer -- `autoprefixer` (peer `^8.1.0`), `postcss-import` (peer `^8.0.0`), and vite's own dependency (`^8.5.15`). Verified: with the override removed a fresh app and the docs app both still resolve exactly one `postcss@8.5.23`, so the security-advisory floor (>= 8.5.18) that motivated the override is held by the pin alone. The override was also strictly worse to carry, which is the real reason to drop it. `update_package_json_overrides` merges framework overrides into a project's `reflex.lock/package.json` and never removes entries, so once an override ships it cannot be retired by a later release -- it becomes a vestigial pin that only user intervention can clear, and one that would actively conflict the next time the `postcss` dev pin moves past the frozen override value. `OVERRIDES` is kept as an empty mapping (the mechanism is still wired up and unit-tested) with a note to prefer a dependency pin whenever the package is one we declare directly. Projects that already installed 0.9.8 keep an inert `"postcss": "8.5.23"` override, confirmed by simulating that upgrade: the persisted entry survives, the build succeeds, and postcss still resolves to 8.5.23. It matches the dev pin, so it is a no-op until removed by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZJDfyvTWhhJ25Lpch4VmS * Scope the react-router v8 no-app-changes claim in the release note The note opened with a blanket "No rxconfig.py or app code changes are needed" and then said custom components wrapping react-router-dom must change their imports, which contradicts it. Scope the claim to apps on the default generated setup and state the react-router-dom import change as required rather than as an aside. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZJDfyvTWhhJ25Lpch4VmS --------- Co-authored-by: Claude <noreply@anthropic.com>
orjson.loads rounds integers outside the 64-bit range to floats, so a customdata id could be silently corrupted when plotly serialized with its stdlib engine.
There was a problem hiding this comment.
2 issues found across 39 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-base/src/reflex_base/.templates/web/utils/state.js">
<violation number="1" location="packages/reflex-base/src/reflex_base/.templates/web/utils/state.js:588">
P2: When a valid socket payload contains a literal `__reflex_nan__` string, this call routes it through the reviver and changes the string to `NaN`. Parse valid JSON without a reviver first, and only use the reviver after detecting and rewriting non-finite tokens or escaped sentinels.</violation>
</file>
<file name="pyproject.toml">
<violation number="1" location="pyproject.toml:68">
P2: The `orjson` optional dependency is declared on `reflex`, but the module that actually imports orjson is `reflex-base` (`packages/reflex-base/src/reflex_base/utils/format.py` does `import orjson` in a try/except fallback). No `orjson` extra is declared in `packages/reflex-base/pyproject.toml`, so reflex-base's published metadata never advertises the optional dependency it owns at runtime. `reflex[orjson]` works because installing reflex pulls reflex-base into the same env, but a consumer who installs reflex-base directly (or who only wants the format helpers) has no supported opt-in and gets a silent stdlib fallback with no way to enable orjson through the package that uses it. Declare the `orjson` extra on reflex-base and have `reflex[orjson]` pull it in (e.g. `reflex-base[orjson]`), so the importing package's metadata is accurate.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } catch (e2) { | ||
| return false; | ||
| } | ||
| return parseNonFiniteAwareJSON(str); |
There was a problem hiding this comment.
P2: When a valid socket payload contains a literal __reflex_nan__ string, this call routes it through the reviver and changes the string to NaN. Parse valid JSON without a reviver first, and only use the reviver after detecting and rewriting non-finite tokens or escaped sentinels.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/.templates/web/utils/state.js, line 588:
<comment>When a valid socket payload contains a literal `__reflex_nan__` string, this call routes it through the reviver and changes the string to `NaN`. Parse valid JSON without a reviver first, and only use the reviver after detecting and rewriting non-finite tokens or escaped sentinels.</comment>
<file context>
@@ -603,16 +585,10 @@ export const connect = async (
- } catch (e2) {
- return false;
- }
+ return parseNonFiniteAwareJSON(str);
+ } catch {
+ // socket.io's decoder expects false for an undecodable packet.
</file context>
There was a problem hiding this comment.
The premise does not hold, and the suggested order would introduce the bug it is guarding against.
Sentinels do not only arrive as bare tokens. The backend emits them as ordinary JSON strings, and escapes any user string that would collide. For {"user": "__reflex_nan__", "v": float("nan")} the wire format is:
{"user":"__reflex_esc____reflex_nan__","v":"__reflex_nan__"}
That is valid JSON. Parsing it without a reviver first succeeds, so the frontend would get the string "__reflex_nan__" instead of NaN, and __reflex_esc____reflex_nan__ would stay unstripped.
The scenario in the comment — an unescaped __reflex_nan__ in a valid payload — cannot occur: everything emitted on this socket goes through orjson_dumps_socket, which escapes collisions before they reach the wire (covered by test_sentinel_string_value_is_escaped and test_escape_prefixed_string_is_escaped_again). The reviver is only reached when the payload contains the prefix at all, which is what keeps the fast parser on the common path.
| "pydantic >=2.12.0,<3.0", | ||
| "sqlmodel >=0.0.24,<0.1", | ||
| ] | ||
| orjson = ["orjson >=3.11.1,<4.0"] |
There was a problem hiding this comment.
P2: The orjson optional dependency is declared on reflex, but the module that actually imports orjson is reflex-base (packages/reflex-base/src/reflex_base/utils/format.py does import orjson in a try/except fallback). No orjson extra is declared in packages/reflex-base/pyproject.toml, so reflex-base's published metadata never advertises the optional dependency it owns at runtime. reflex[orjson] works because installing reflex pulls reflex-base into the same env, but a consumer who installs reflex-base directly (or who only wants the format helpers) has no supported opt-in and gets a silent stdlib fallback with no way to enable orjson through the package that uses it. Declare the orjson extra on reflex-base and have reflex[orjson] pull it in (e.g. reflex-base[orjson]), so the importing package's metadata is accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pyproject.toml, line 68:
<comment>The `orjson` optional dependency is declared on `reflex`, but the module that actually imports orjson is `reflex-base` (`packages/reflex-base/src/reflex_base/utils/format.py` does `import orjson` in a try/except fallback). No `orjson` extra is declared in `packages/reflex-base/pyproject.toml`, so reflex-base's published metadata never advertises the optional dependency it owns at runtime. `reflex[orjson]` works because installing reflex pulls reflex-base into the same env, but a consumer who installs reflex-base directly (or who only wants the format helpers) has no supported opt-in and gets a silent stdlib fallback with no way to enable orjson through the package that uses it. Declare the `orjson` extra on reflex-base and have `reflex[orjson]` pull it in (e.g. `reflex-base[orjson]`), so the importing package's metadata is accurate.</comment>
<file context>
@@ -65,6 +65,7 @@ db = [
"pydantic >=2.12.0,<3.0",
"sqlmodel >=0.0.24,<0.1",
]
+orjson = ["orjson >=3.11.1,<4.0"]
pydantic = ["reflex-base[pydantic]"]
</file context>
|
blocked by #6933 edit: merged/unblocked now |
No description provided.