fix: ensure each createFormControl.subscribe subscription listens only to the changes it subscribes to#12968
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR isolates subscription filters so that each subscribe call only listens to the specific formState keys it requested, preventing subscriptions from unintentionally accumulating filters.
- Introduce
defaultProxyFormStateas the base filter set for every subscription - Rename and reinitialize
_proxyFormStatefor clarity - Update
subscribeto merge onlydefaultProxyFormStatewith the incomingprops.formState - Add a test to verify multiple subscriptions trigger only their own callbacks
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/logic/createFormControl.ts | Define defaultProxyFormState and use it in each subscribe call to reset filters |
| src/tests/useForm/subscribe.test.tsx | Add a test ensuring distinct subscriptions only fire for their own field changes |
Comments suppressed due to low confidence (2)
src/logic/createFormControl.ts:154
- [nitpick] The name
defaultProxyFormStatecould be more descriptive or follow the existing private naming pattern (e.g.,_defaultProxyFormStateorinitialProxyFormState) to align with other variables in this module.
const defaultProxyFormState: ReadFormState = {
src/tests/useForm/subscribe.test.tsx:10
- [nitpick] Consider renaming the mock callbacks (e.g.,
firstCallback,secondCallback) for clearer intent in the test.
const cb2 = jest.fn();
| formState: { | ||
| ...defaultProxyFormState, | ||
| ...props.formState, | ||
| }, |
There was a problem hiding this comment.
Do we still need this defaultProxyFormState?
what's the diff if we just do:
..._proxyFormState,
...props.formState,
There was a problem hiding this comment.
I think we do still need defaultProxyFormState. The example below also mutates _proxyFormState, so we may end up subscribing to changes in formState that we didn't request in the subscribe call.
const fieldFormState = useFormState({
name: "field1",
control,
});
fieldFormState.touchedFields
Do you want me to create test for this?
There was a problem hiding this comment.
Do you want me to create test for this?
yes plz <3
Not sure if its a bug fix or a feature. When you register multiple subscriptions, their formState filters are merged, so the last called subscription will listen on updates for both its own fields and all fields subscribed to by earlier subscriptions.
Example: https://codesandbox.io/p/sandbox/vwfnsk?file=%2Fsrc%2FApp.tsx%3A5%2C18
Steps:
This PR ensures that each subscription listens only to the changes it subscribes to.