Reject browser storage defaults produced by a default_factory

[masenf]

Requested by Masen · project thread

All Submissions:

• Have you followed the guidelines stated in CONTRIBUTING.md file?
• Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Changes To Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your core changes, as applicable?
• Have you successfully ran tests with your changes locally?

What

Before: a str-annotated var whose default_factory returned a LocalStorage was never detected as browser storage. It compiled as an ordinary var and each session silently held a storage instance as a plain str. A storage-annotated var with a factory was accepted, and compile_client_storage called the factory to read the storage name and options.

After: both declarations fail at compile time with a VarTypeError naming the var and showing the supported form (theme: str = rx.LocalStorage("", name='theme')). Browser storage must be declared with a storage value as the default or with a storage type annotation. The docs say so in Browser Storage and in Changing Defaults.

This rejects rather than warns: a factory-produced storage value cannot be classified when the app compiles, so there is no working fallback to warn about, and nothing in the codebase deprecates a declaration that never worked.

How

_compile_client_storage_field raises when a storage-annotated field has no default (a declared factory, or one set later with set_default(default_factory=...)), and when a field whose annotation accepts a str (str, str | None, Any, object) has a factory that returns a ClientStorageBase. That second check calls the factory once at compile for str-compatible factory fields only; factories for other types are never called. The special case that read storage options from a factory's product is removed, along with the unit and Playwright tests that exercised factory-declared storage.

Test plan

• tests/units/compiler/test_compiler_utils.py: rejection for each storage type via annotation, via set_default(default_factory=...), and for str / str | None factories returning storage; a plain str factory and a list factory compile as ordinary vars with the str factory called once.
• uv run pytest tests/units/compiler tests/units/test_state.py tests/units/utils/test_telemetry_accounting.py: 638 passed.
• uv run pytest tests/integration/tests_playwright/test_hydration_storage.py: 32 passed.
• ruff check, ruff format --check, pyright reflex tests: clean.

Closes #7498

🤖 Generated with Claude Code

https://claude.ai/code/session_014uPh1ADyPWucA2MM6zpW9T

---

Generated by Claude Code

[greptile-apps]

Confidence Score: 5/5

[Critical impact] The PR appears safe to merge; both previous findings are fixed and no new actionable issues remain.

Summary

The PR rejects browser storage declared through a default_factory and documents the supported forms.

• Browser-storage fields with factory defaults now fail at compile time.

_{Reviews (2) · Last reviewed commit: "Leave backend factories alone; catch sto..." · Reviewed by Greptile}

[codspeed]

Merging this PR will degrade performance by 4.7%

⚠️ 19 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

• test_cli_startup[run_help]
• test_cli_startup[cloud_help]
• test_cli_startup[deploy_help]
• test_event_future_tree
• test_yield_intermediate_update
• test_event_shutdown_cancel
• test_get_state_uncached[disk]
• test_state_manager_memory_read_only_modify
• test_state_manager_memory_cold_get
• test_event_queue_dispatch
• …and 9 more

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 147 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	test_state_access[write_own]	126.1 ms	132.3 ms	-4.7%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing claude/reject-factory-storage-defaults-1uthtm (248a484) with main (5ae5dcd)}

[masenf]

CodSpeed Performance Analysis is red on this head, flagging test_state_access[write_own] at -4.7%. On the previous head it flagged test_state_access[read_own] instead, and both runs carry CodSpeed's "Different runtime environments detected" warning. The PR only changes compile_client_storage, a compile-time path that tests/benchmarks/test_state_access.py never calls, so the regression is measurement noise rather than this change. Nothing to port; leaving it as is.

---

Generated by Claude Code