Support TypedDict form data in on_submit#6301
Conversation
Signed-off-by: Gautam Manchandani <gautammanch@Gautams-MacBook-Air.local>
Merging this PR will not alter performance
Comparing Footnotes
|
Greptile SummaryThis PR adds compile-time
Confidence Score: 4/5Safe to merge after fixing the The static validation logic is well-designed and the tests are thorough. The one concrete defect is in packages/reflex-components-core/src/reflex_components_core/el/elements/forms.py — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Form.create(*children, on_submit=handler)"] --> B["_validate_on_submit_typed_dict_fields()"]
B --> C{"on_submit is EventChain?"}
C -- No --> Z["Skip validation"]
C -- Yes --> D["Iterate events in chain"]
D --> E{"event is EventSpec?"}
E -- No --> Z
E -- Yes --> F{"args include FORM_DATA?"}
F -- No --> D
F -- Yes --> G["Extract handler.fn (unwrap partial)"]
G --> H["get_type_hints(func)"]
H --> I{"annotation is TypedDict?"}
I -- No --> D
I -- Yes --> J["Collect required_fields via _get_required_typed_dict_fields()"]
J --> D
D --> K{"typed_dict_contracts empty?"}
K -- Yes --> Z
K -- No --> L{"form has id?"}
L -- Yes --> Z
L -- No --> M["_get_static_form_field_keys()"]
M --> N["Collect form refs + _is_form_control name/id props"]
N --> O{"All required fields present?"}
O -- Yes --> P["✓ Form created OK"]
O -- No --> Q{"has_dynamic_identifiers?"}
Q -- Yes --> P
Q -- No --> R["✗ raise EventHandlerValueError"]
Reviews (2): Last reviewed commit: "update pyi_files" | Re-trigger Greptile |
|
Can you please add these tests as well and fix the issue? def test_on_submit_accepts_typed_dict_with_inherited_optional_fields():
"""Inherited optional TypedDict keys should remain optional."""
class BaseSignupData(TypedDict, total=False):
nickname: str
class SignupData(BaseSignupData):
email: str
class SignupState(rx.State):
@rx.event
def on_submit(self, form_data: SignupData):
pass
form = HTMLForm.create(
Input.create(name="email"),
on_submit=SignupState.on_submit,
)
assert isinstance(form.event_triggers["on_submit"], EventChain)def test_on_submit_accepts_controls_associated_via_form_attribute():
"""Controls associated via the HTML form attribute should not fail validation."""
class SignupData(TypedDict):
email: str
class SignupState(rx.State):
@rx.event
def on_submit(self, form_data: SignupData):
pass
form = HTMLForm.create(
id="signup",
on_submit=SignupState.on_submit,
)
Input.create(name="email", form="signup")
assert isinstance(form.event_triggers["on_submit"], EventChain) |
Use __required_keys__ for inherited TypedDict optional fields, skip validation for forms with id (HTML form attribute), replace stringly-typed control detection with _is_form_control marker, use concrete Mapping type in event spec, narrow exception handling, and add integration tests.
On 3.10, typing.TypedDict ignores typing_extensions.NotRequired when populating __required_keys__. Fall back to annotation inspection to subtract NotRequired fields on older Python versions.
…tamBytes/reflex into feat/support-typeddict-forms
Unit tests now pair happy paths with failing counterparts to prove validation is active. Integration test carries input/expected data per variant instead of fragile runtime app_source detection.
| return isinstance(value, Var) and value._js_expr == FORM_DATA._js_expr | ||
|
|
||
|
|
||
| def _get_handler_name(handler: EventHandler) -> str: |
There was a problem hiding this comment.
I think this can be inlined. As I understand it is called once.
masenf
left a comment
There was a problem hiding this comment.
update from main.
this is a nice feature. i think the new iter_form_fields functionality could be used to replace _get_all_refs in the Form implementation which is the only place where that is really used. We could get rid of that relatively shaky method.
will also have to make sure this continues to work when the form's children are memoized components (have event handlers or other state associated with them). update the integration test to make sure at least one of the fields depends on state and thus will get auto-memoized.
|
@greptile-apps re-review this pr |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
All Submissions:
Type of change
New Feature Submission:
Changes To Core Features:
Description
This PR adds
TypedDictsupport foron_submitform data handlers while keeping existingdict[str, Any]anddict[str, str]handlers working as before.What changed:
on_submithandlers annotated with a concreteTypedDictTypedDictkeys against statically knowable form fields at form construction timenamefields and existing id-backed form refs in the validation setThis improves:
closes #6264