Skip to content

Commit

Permalink
fix(forms): make Form.Isolations data flow "from outside" stricter (#…
Browse files Browse the repository at this point in the history
…3847)

Fixes #3844

The test fails with the old implementation 👍
  • Loading branch information
tujoworker authored Aug 20, 2024
1 parent 0fe23a9 commit 977962b
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ It's a provider that lets you provide a `schema` or `data` very similar to what
### Good to know

- It needs to be used inside of a `Form.Handler` component.
- All fields inside needs to validate successfully before the isolated data can be commited, like the `Form.Handler` does before it submits.
- All fields inside need to validate successfully before the isolated data can be committed, just like the `Form.Handler` does before submitting.
- Input fields are prevented from submitting the form when pressing enter. Pressing enter on input fields will commit the isolated data to the `Form.Handler` context instead.
- You can provide a `schema`, `data` or `defaultData` like you would do with the `Form.Handler`.
- You can also provide `data` or `defaultData` to the `Form.Handler`, defining the data that will be used for the isolated data.
- You can also provide `data` or `defaultData` to the `Form.Handler` component. If not given on the `Form.Isolation` component, this will define the data that will be used for the isolated data.
- Using `Form.Isolation` inside of a `Form.Section` is supported.
- `onChange` on the `Form.Handler` will be called when the isolated data gets commited.
- `onChange` on the `Form.Isolation` will be called on every change of the isolated data. Use `onCommit` to get the data that gets commited.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export interface ContextState {
setValueProps?: (path: Path, props: any) => void
fieldPropsRef?: React.MutableRefObject<Record<string, FieldProps>>
valuePropsRef?: React.MutableRefObject<Record<string, ValueProps>>
mountedFieldPathsRef?: React.MutableRefObject<Path[]>
showAllErrors: boolean
hasVisibleError: boolean
formState: SubmitState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -892,17 +892,7 @@ export default function Provider<Data extends JsonObject>(

try {
if (isolate) {
const mounterData = {} as Data
mountedFieldPathsRef.current.forEach((path) => {
if (pointer.has(internalDataRef.current, path)) {
pointer.set(
mounterData,
path,
pointer.get(internalDataRef.current, path)
)
}
})
result = await onCommit?.(mounterData, {
result = await onCommit?.(internalDataRef.current, {
clearData,
})
} else {
Expand Down Expand Up @@ -1150,6 +1140,7 @@ export default function Provider<Data extends JsonObject>(
Object.keys(hasVisibleErrorRef.current).length > 0,
fieldPropsRef,
valuePropsRef,
mountedFieldPathsRef,
ajvInstance: ajvRef.current,

/** Additional */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, {
useCallback,
useContext,
useEffect,
useMemo,
useReducer,
useRef,
} from 'react'
import pointer, { JsonObject } from 'json-pointer'
import { extendDeep } from '../../../../shared/component-helper'
import { Context, Provider } from '../../DataContext'
import { Context, ContextState, Provider } from '../../DataContext'
import SectionContext from '../Section/SectionContext'
import IsolationCommitButton from './IsolationCommitButton'
import {
Expand Down Expand Up @@ -82,6 +83,7 @@ function IsolationProvider<Data extends JsonObject>(
const [, forceUpdate] = useReducer(() => ({}), {})
const internalDataRef = useRef<Data>()
const localDataRef = useRef<Partial<Data>>({})
const dataContextRef = useRef<ContextState>(null)
const outerContext = useContext(Context)
const { path: pathSection } = useContext(SectionContext) || {}
const { handlePathChange: handlePathChangeOuter, data: dataOuter } =
Expand Down Expand Up @@ -113,6 +115,22 @@ function IsolationProvider<Data extends JsonObject>(
[pathSection]
)

const getMountedData = useCallback((data: Data) => {
const mounterData = {} as Data
dataContextRef.current?.mountedFieldPathsRef.current.forEach(
(path) => {
if (pointer.has(data, path)) {
pointer.set(mounterData, path, pointer.get(data, path))
}
}
)
return mounterData
}, [])

useEffect(() => {
localDataRef.current = getMountedData(internalDataRef.current)
}, [getMountedData])

// Update the isolated data with the outside context data
useMemo(() => {
if (localDataRef.current === clearedData) {
Expand All @@ -131,16 +149,16 @@ function IsolationProvider<Data extends JsonObject>(
localData = obj
}

internalDataRef.current = extendDeep(
internalDataRef.current = Object.assign(
{},
dataOuter,
localData || {},
localData || dataOuter || {},
localDataRef.current
) as Data
)
}, [data, defaultData, dataOuter, pathSection])

const onCommit: IsolationProps<Data>['onCommit'] = useCallback(
async (mountedData: Data, additionalArgs) => {
async (data: Data, additionalArgs) => {
const mountedData = getMountedData(data)
const path = props.path ?? '/'
const outerData =
props.path && pointer.has(dataOuter, path)
Expand All @@ -166,6 +184,7 @@ function IsolationProvider<Data extends JsonObject>(
)
},
[
getMountedData,
props.path,
dataOuter,
transformOnCommitProp,
Expand Down Expand Up @@ -196,6 +215,8 @@ function IsolationProvider<Data extends JsonObject>(
<Provider {...providerProps}>
<Context.Consumer>
{(dataContext) => {
dataContextRef.current = dataContext

if (commitHandleRef) {
commitHandleRef.current = dataContext?.handleSubmit
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ describe('Form.Isolation', () => {
expect(isolated).toHaveValue('Isolated updated')
})

it('should update data when changed in the root context', async () => {
it('should not change the isolated value when changed in the root context', async () => {
render(
<Form.Handler id="form-id" data={{ isolated: 'Isolated' }}>
<Field.String path="/isolated" />
Expand All @@ -384,7 +384,60 @@ describe('Form.Isolation', () => {
})

expect(outside).toHaveValue('Changed')
expect(inside).toHaveValue('Changed')
expect(inside).toHaveValue('Isolated')
})

it('should not override isolated values inside a section when changed via the root context', async () => {
const data = {
section: {
first: 'First',
second: 'Second',
},
}

const MockComponent = () => {
const [state, setState] = React.useState(data.section.second)
return (
<Form.Handler
data={{
section: {
first: 'First',
second: 'Second',
},
}}
>
<Form.Section path="/section">
<Form.Isolation defaultData={{ first: 'First value' }}>
<Field.String path="/first" />
<Field.String
path="/second"
value={state}
onChange={(value) => {
setState(value)
}}
/>
</Form.Isolation>
</Form.Section>
</Form.Handler>
)
}

render(<MockComponent />)

const [first, second] = Array.from(document.querySelectorAll('input'))

expect(first).toHaveValue('First value')
expect(second).toHaveValue('Second')

await userEvent.type(second, ' changed')

expect(first).toHaveValue('First value')
expect(second).toHaveValue('Second changed')

await userEvent.type(first, ' changed')

expect(first).toHaveValue('First value changed')
expect(second).toHaveValue('Second changed')
})

it('should not call onChange on root context', async () => {
Expand Down

0 comments on commit 977962b

Please sign in to comment.