Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid notify listener calls on field register when there's no subscriber #319

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/FinalForm.fieldSubscribing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -888,4 +888,63 @@ describe('Field.subscribing', () => {
expect(name1).toHaveBeenCalledTimes(1)
expect(name2).toHaveBeenCalledTimes(1)
})

it('should subscribe and unsubscribe to field state', () => {
const form = createForm({ onSubmit: onSubmitMock })
const foo = jest.fn()

form.registerField('foo')
const unsubscribe = form.subscribeToFieldState('foo', foo, {
touched: true,
value: true,
visited: true
})

expect(foo).not.toHaveBeenCalled()

form.change('foo', 'new value')

expect(foo).toHaveBeenCalledTimes(1)
expect(foo.mock.calls[0][0].value).toBe('new value')

form.focus('foo')
expect(foo).toHaveBeenCalledTimes(2)
expect(foo.mock.calls[1][0].touched).toBe(false)
expect(foo.mock.calls[1][0].visited).toBe(true)

form.blur('foo')
expect(foo).toHaveBeenCalledTimes(3)
expect(foo.mock.calls[2][0].touched).toBe(true)
expect(foo.mock.calls[2][0].visited).toBe(true)

unsubscribe()
form.change('foo', 'newer value')
form.focus('foo')
form.blur('foo')
expect(foo).toHaveBeenCalledTimes(3)
})

it('should subscribe and unsubscribe to field state with only value subscription', () => {
const form = createForm({ onSubmit: onSubmitMock })
const foo = jest.fn()

form.registerField('foo')
const unsubscribe = form.subscribeToFieldState('foo', foo, { value: true })

expect(foo).not.toHaveBeenCalled()

form.change('foo', 'new value')

expect(foo).toHaveBeenCalledTimes(1)
expect(foo.mock.calls[0][0].value).toBe('new value')

form.focus('foo')
expect(foo).toHaveBeenCalledTimes(1)
form.blur('foo')
expect(foo).toHaveBeenCalledTimes(1)

unsubscribe()
form.change('foo', 'newer value')
expect(foo).toHaveBeenCalledTimes(1)
})
})
96 changes: 74 additions & 22 deletions src/FinalForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ type InternalState<FormValues> = {
[string]: InternalFieldState
},
fieldSubscribers: { [string]: Subscribers<FieldState> },
formState: InternalFormState<FormValues>
formState: InternalFormState<FormValues>,
registeredFieldCounts: { [string]: number }
} & MutableState<FormValues>

export type StateFilter<T> = (
Expand Down Expand Up @@ -200,7 +201,8 @@ function createForm<FormValues: FormValuesShape>(
validating: 0,
values: initialValues ? { ...initialValues } : (({}: any): FormValues)
},
lastFormState: undefined
lastFormState: undefined,
registeredFieldCounts: {}
}
let inBatch = 0
let validationPaused = false
Expand Down Expand Up @@ -651,6 +653,39 @@ function createForm<FormValues: FormValuesShape>(
name => state.fields[name].afterSubmit && state.fields[name].afterSubmit()
)

const subscribeToFieldState = (
name: string,
subscriber: FieldSubscriber,
subscription: FieldSubscription = {},
notified
): Unsubscribe => {
if (state.fields[name] === undefined) {
throw Error(
'You must register ' + name + ' field before you can subscribe to it!'
)
}

if (!state.fieldSubscribers[name]) {
state.fieldSubscribers[name] = { index: 0, entries: {} }
}
const index = state.fieldSubscribers[name].index++

// save field subscriber callback
state.fieldSubscribers[name].entries[index] = {
subscriber: memoize(subscriber),
subscription,
notified
}

return () => {
delete state.fieldSubscribers[name].entries[index]
let lastOne = !Object.keys(state.fieldSubscribers[name].entries).length
if (lastOne) {
delete state.fieldSubscribers[name]
}
}
}

const resetModifiedAfterSubmit = (): void =>
Object.keys(state.fields).forEach(
key => (state.fields[key].modifiedSinceLastSubmit = false)
Expand Down Expand Up @@ -797,21 +832,14 @@ function createForm<FormValues: FormValuesShape>(

registerField: (
name: string,
subscriber: FieldSubscriber,
subscription: FieldSubscription = {},
subscriber?: FieldSubscriber,
subscription?: FieldSubscription = {},
fieldConfig?: FieldConfig
): Unsubscribe => {
if (!state.fieldSubscribers[name]) {
state.fieldSubscribers[name] = { index: 0, entries: {} }
}
const index = state.fieldSubscribers[name].index++

// save field subscriber callback
state.fieldSubscribers[name].entries[index] = {
subscriber: memoize(subscriber),
subscription,
notified: false
if (state.registeredFieldCounts[name] === undefined) {
state.registeredFieldCounts[name] = 0
}
const index = ++state.registeredFieldCounts[name]

if (!state.fields[name]) {
// create initial field state
Expand All @@ -835,7 +863,20 @@ function createForm<FormValues: FormValuesShape>(
validating: false,
visited: false
}

if (subscriber === undefined) {
state.fields[name].lastFieldState = publishFieldState(
state.formState,
state.fields[name]
)
}
}

// subscribe or don't
const unsubscribeToFieldState =
subscriber &&
subscribeToFieldState(name, subscriber, subscription, false)

let haveValidator = false
const silent = fieldConfig && fieldConfig.silent
const notify = () => {
Expand Down Expand Up @@ -891,6 +932,13 @@ function createForm<FormValues: FormValuesShape>(
}

if (haveValidator) {
runValidation(undefined, () => {
notifyFormListeners()
notifyFieldListeners()
})
} else if (subscriber !== undefined) {
notifyFormListeners()
notifyFieldListeners(name)
runValidation(undefined, notify)
} else {
notify()
Expand All @@ -907,13 +955,10 @@ function createForm<FormValues: FormValuesShape>(
)
delete state.fields[name].validators[index]
}
let hasFieldSubscribers = !!state.fieldSubscribers[name];
if (hasFieldSubscribers) {
// state.fieldSubscribers[name] may have been removed by a mutator
delete state.fieldSubscribers[name].entries[index]
}
let lastOne = hasFieldSubscribers && !Object.keys(state.fieldSubscribers[name].entries).length
if(unsubscribeToFieldState) unsubscribeToFieldState()
let lastOne = --state.registeredFieldCounts[name] === 0
if (lastOne) {
delete state.registeredFieldCounts[name]
delete state.fieldSubscribers[name]
delete state.fields[name]
if (validatorRemoved) {
Expand All @@ -931,7 +976,7 @@ function createForm<FormValues: FormValuesShape>(
notifyFormListeners()
notifyFieldListeners()
})
} else if (lastOne) {
} else if (lastOne && subscriber !== undefined) {
// values or errors may have changed
notifyFormListeners()
}
Expand Down Expand Up @@ -1188,7 +1233,14 @@ function createForm<FormValues: FormValuesShape>(
return () => {
delete subscribers.entries[index]
}
}
},

subscribeToFieldState: (
name: string,
subscriber: FieldSubscriber,
subscription: FieldSubscription = {}
): Unsubscribe =>
subscribeToFieldState(name, subscriber, subscription, true)
}
return api
}
Expand Down
7 changes: 6 additions & 1 deletion src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export type Subscribers<T extends Object> = {
subscriber: Subscriber<T>
subscription: Subscription
notified: boolean
}
} | void
}
}

Expand Down Expand Up @@ -228,6 +228,11 @@ export interface FormApi<FormValues = Record<string, any>, InitialFormValues = P
subscriber: FormSubscriber<FormValues>,
subscription: FormSubscription
) => Unsubscribe
subscribeToExistingField: (
name: string,
subscriber: FieldSubscriber<any>,
subscription: FieldSubscription
) => Unsubscribe
}

export type DebugFunction<FormValues, InitialFormValues = Partial<FormValues>> = (
Expand Down
11 changes: 8 additions & 3 deletions src/types.js.flow
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export type FieldSubscriber = Subscriber<FieldState>
export type Subscribers<T: Object> = {
index: number,
entries: {
[number]: {
[number]: ?{
subscriber: Subscriber<T>,
subscription: Subscription,
notified: boolean
Expand Down Expand Up @@ -151,8 +151,8 @@ export type FieldConfig = {

export type RegisterField = (
name: string,
subscriber: FieldSubscriber,
subscription: FieldSubscription,
subscriber?: FieldSubscriber,
subscription?: FieldSubscription,
config?: FieldConfig
) => Unsubscribe

Expand Down Expand Up @@ -232,6 +232,11 @@ export type FormApi<FormValues: FormValuesShape> = {
subscribe: (
subscriber: FormSubscriber<FormValues>,
subscription: FormSubscription
) => Unsubscribe,
subscribeToFieldState: (
name: string,
subscriber: FieldSubscriber,
subscription: FieldSubscription
) => Unsubscribe
}

Expand Down