Skip to content

Commit

Permalink
Make flattenSubscription hooks-safe (#479)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andarist authored and erikras committed May 16, 2019
1 parent 6054da7 commit 4e72442
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 26 deletions.
11 changes: 5 additions & 6 deletions src/ReactFinalForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const ReactFinalForm = ({
keepDirtyOnReinitialize,
mutators,
onSubmit,
subscription,
subscription = all,
validate,
validateOnBlur,
...rest
Expand Down Expand Up @@ -74,14 +74,13 @@ const ReactFinalForm = ({
let initialState: FormState = {}
form.subscribe(state => {
initialState = state
}, subscription || all)()
}, subscription)()
return initialState
}
)

// ⚠️ flattenedSubscription is probably not "hook-safe".
// In the future, changing subscriptions on the fly should be banned. ⚠️
const flattenedSubscription = flattenSubscription(subscription || all)
const flattenedSubscription = flattenSubscription(subscription)
React.useEffect(() => {
// We have rendered, so all fields are no registered, so we can unpause validation
form.isValidationPaused() && form.resumeValidation()
Expand All @@ -90,7 +89,7 @@ const ReactFinalForm = ({
if (!shallowEqual(s, state)) {
setState(s)
}
}, subscription || all),
}, subscription),
...(decorators
? decorators.map(decorator =>
// this noop ternary is to appease the flow gods
Expand All @@ -104,7 +103,7 @@ const ReactFinalForm = ({
unsubscriptions.forEach(unsubscribe => unsubscribe())
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [decorators, ...flattenedSubscription])
}, [decorators, flattenedSubscription])

// warn about decorator changes
// istanbul ignore next
Expand Down
8 changes: 6 additions & 2 deletions src/flattenSubscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
type Subscription = { [string]: boolean }
export default function flattenSubscription(
subscription: Subscription = {}
): string[] {
return Object.keys(subscription).filter(key => subscription[key] === true)
): string {
return Object.keys(subscription)
.filter(key => subscription[key] === true)
.map(key => key)
.sort()
.join(',')
}
13 changes: 6 additions & 7 deletions src/flattenSubscription.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ import flattenSubscription from './flattenSubscription'

describe('flattenSubscription', () => {
it('should return empty array when subscription is empty', () => {
expect(flattenSubscription(undefined)).toEqual([])
expect(flattenSubscription({})).toEqual([])
expect(flattenSubscription(undefined)).toEqual('')
expect(flattenSubscription({})).toEqual('')
})

it('should return only keys that are true in subscription', () => {
expect(flattenSubscription({ foo: true, bar: false })).toEqual(['foo'])
expect(flattenSubscription({ foo: true, bar: true, baz: false })).toEqual([
'foo',
'bar'
])
expect(flattenSubscription({ foo: true, bar: false })).toEqual('foo')
expect(flattenSubscription({ foo: true, bar: true, baz: false })).toEqual(
'bar,foo'
)
})
})
9 changes: 4 additions & 5 deletions src/useField.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const useField = (
isEqual,
multiple,
parse = defaultParse,
subscription,
subscription = all,
type,
validate,
validateFields,
Expand All @@ -49,7 +49,7 @@ const useField = (

const beforeSubmitRef = React.useRef()
const register = (callback: FieldState => void) =>
form.registerField(name, callback, subscription || all, {
form.registerField(name, callback, subscription, {
afterSubmit,
beforeSubmit: () => beforeSubmitRef.current && beforeSubmitRef.current(),
defaultValue,
Expand Down Expand Up @@ -82,9 +82,8 @@ const useField = (
return beforeSubmit && beforeSubmit()
}

// ⚠️ flattenedSubscription is probably not "hook-safe".
// In the future, changing subscriptions on the fly should be banned. ⚠️
const flattenedSubscription = flattenSubscription(subscription || all)
const flattenedSubscription = flattenSubscription(subscription)
React.useEffect(
() =>
register(state => {
Expand All @@ -105,7 +104,7 @@ const useField = (
isEqual,
validateFields,
// eslint-disable-next-line react-hooks/exhaustive-deps
...flattenedSubscription
flattenedSubscription
]
)

Expand Down
11 changes: 5 additions & 6 deletions src/useFormState.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import useForm from './useForm'

const useFormState = ({
onChange,
subscription
subscription = all
}: UseFormStateParams = {}): FormState => {
const form: FormApi = useForm('useFormState')
const firstRender = React.useRef(true)
Expand All @@ -19,17 +19,16 @@ const useFormState = ({
let initialState: FormState = {}
form.subscribe(state => {
initialState = state
}, subscription || all)()
}, subscription)()
if (onChange) {
onChange(initialState)
}
return initialState
}
)

// ⚠️ flattenedSubscription is probably not "hook-safe".
// In the future, changing subscriptions on the fly should be banned. ⚠️
const flattenedSubscription = flattenSubscription(subscription || all)
const flattenedSubscription = flattenSubscription(subscription)
React.useEffect(
() =>
form.subscribe(newState => {
Expand All @@ -41,9 +40,9 @@ const useFormState = ({
onChange(newState)
}
}
}, subscription || all),
}, subscription),
// eslint-disable-next-line react-hooks/exhaustive-deps
flattenedSubscription
[flattenedSubscription]
)
return state
}
Expand Down

0 comments on commit 4e72442

Please sign in to comment.