Validate reserved state names before registration - #7136
FarhanAliRaza wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
| for name, field in BaseState.__fields__.items(): | ||
| if field.is_var: | ||
| members.pop(name, None) |
There was a problem hiding this comment.
_reserved_state_members() removes every field marked is_var, but BaseState.router is a framework-owned RouterData field with that flag. A subclass can therefore declare an ordinary router field without being rejected. Instances then initialize router with the user value, causing framework accesses such as self.router._page, self.router.session, or self.router.url to fail with an attribute or type error.
Merging this PR will not alter performance
Comparing Footnotes
|
State vars named
_get_was_touchedor_update_was_touchedcan replace framework helpers and crash persistence or cleanup. Other collisions, includingget_fieldsanddirty_vars, can break class creation or bookkeeping. This rejects reserved declarations before field collection and state initialization, and applies the same validation to dynamic vars, fields, event handlers, and route arguments.Reserved names are derived once from
BaseStateand its framework bases. Ordinary vars, inherited user vars, Python protocols, and explicitly marked method overrides remain supported. Validation also checks state mixins, ordinary Python mixins, and inherited model fields without invoking descriptors.This changes the default behavior. Existing apps can temporarily set
REFLEX_STATE_ALLOW_RESERVED_NAMES=1to retain legacy handling of conflicting vars with a deprecation warning until 1.0. The flag preserves the old behavior, including collision-related crashes; renaming conflicting declarations resolves them. Migration guidance and fragments for both affected packages are included.Supersedes #7132 and the method-specific follow-up fix for #7135.
Closes #7091.
Closes #7135.
Validation:
All 35 new cases pass; on the unchanged base, the 33 regression cases fail and the two compatibility controls pass.
The final state suites pass all 312 tests.
Full unit suite: 8,626 passed, 18 skipped. The coverage gate exits nonzero at 71.34%, below the required 72%; the unchanged base also reports 71.34% with 8,591 passed and 18 skipped. The new validator has 97.59% coverage; state plus validation have 86.19%.
Full
pyright reflex testspasses. All commit hooks pass, including Ruff, codespell, stub generation, Pyright, and ty.Repository Ruff checks and formatting pass with the ignored
ignore/scratch tree excluded.Followed CONTRIBUTING.md and added regression tests, migration documentation, and package news fragments.
Breaking change with a temporary compatibility opt-in.
Checked overlapping work: this is the centralized replacement for Call _get_was_touched through BaseState so a state var cannot shadow it #7132 and the narrow A backend var named _update_was_touched breaks state cleanup and persistence checks #7135 follow-up.