Skip to content

Commit

Permalink
fix(Forms): avoid unnecessary rerenders in Form.Handler (#4363)
Browse files Browse the repository at this point in the history
The reason behind this is that I saw a negative side-effect in PR #4357
when it comes to the initial render, where we don't want an animation,
but it did animate.
  • Loading branch information
tujoworker authored Dec 5, 2024
1 parent 52f8b2f commit 7de5e49
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ export default function Provider<Data extends JsonObject>(
(Array.isArray(internalDataRef.current) ? [] : clearedData)) as Data

if (id) {
setSharedData?.(internalDataRef.current)
setSharedData(internalDataRef.current)
}

forceUpdate()
Expand All @@ -731,29 +731,28 @@ export default function Provider<Data extends JsonObject>(

useLayoutEffect(() => {
// Set the shared state, if initialData was given
if (id && initialData && !sharedData.data) {
extendSharedData?.(initialData)
if (id) {
if (initialData && !sharedData.data) {
extendSharedData(initialData, { preventSyncOfSameInstance: true })
}
}
}, [id, initialData, extendSharedData, sharedData.data])

useMemo(() => {
executeAjvValidator()

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [internalDataRef.current]) // run validation when internal data has changed

useLayoutEffect(() => {
if (id) {
extendAttachment?.({
visibleDataHandler,
filterDataHandler,
hasErrors,
hasFieldError,
setShowAllErrors,
setSubmitState,
clearData,
fieldConnectionsRef,
})
extendAttachment(
{
visibleDataHandler,
filterDataHandler,
hasErrors,
hasFieldError,
setShowAllErrors,
setSubmitState,
clearData,
fieldConnectionsRef,
},
{ preventSyncOfSameInstance: true }
)
if (filterSubmitData) {
rerenderUseDataHook?.()
}
Expand All @@ -770,8 +769,15 @@ export default function Provider<Data extends JsonObject>(
setShowAllErrors,
setSubmitState,
clearData,
extendSharedData,
])

useMemo(() => {
executeAjvValidator()

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [internalDataRef.current]) // run validation when internal data has changed

const storeInSession = useMemo(() => {
return debounce(
() => {
Expand All @@ -795,7 +801,7 @@ export default function Provider<Data extends JsonObject>(

if (id) {
// Will ensure that Form.getData() gets the correct data
extendSharedData?.(newData)
extendSharedData(newData, { preventSyncOfSameInstance: true })
if (filterSubmitData) {
rerenderUseDataHook?.()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3423,12 +3423,8 @@ describe('DataContext.Provider', () => {
</DataContext.Provider>
)

expect(nestedMockData).toHaveLength(3)
expect(nestedMockData).toEqual([
initialData,
initialData,
initialData,
])
expect(nestedMockData).toHaveLength(2)
expect(nestedMockData).toEqual([initialData, initialData])

const inputElement = document.querySelector('input')
expect(inputElement).toHaveValue('bar')
Expand Down Expand Up @@ -3462,12 +3458,8 @@ describe('DataContext.Provider', () => {
</>
)

expect(sidecarMockData).toHaveLength(3)
expect(sidecarMockData).toEqual([
undefined,
initialData,
initialData,
])
expect(sidecarMockData).toHaveLength(2)
expect(sidecarMockData).toEqual([undefined, initialData])

expect(nestedMockData).toHaveLength(2)
expect(nestedMockData).toEqual([initialData, initialData])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,9 @@ function WizardContainer(props: Props) {
// - Handle shared state
useLayoutEffect(() => {
if (id && hasContext) {
sharedStateRef.current?.extend?.(providerValue)
sharedStateRef.current.extend(providerValue)
}
}, [id, providerValue]) // eslint-disable-line react-hooks/exhaustive-deps
}, [hasContext, id, providerValue])

useLayoutEffect(() => {
updateTitlesRef.current?.()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,37 @@ describe('useSharedState', () => {
expect(resultA.current.data).toEqual({ foo: 'baz' })
expect(resultB.current.data).toEqual({ foo: 'baz' })
})

it('should sync all hooks, except the one that is set to "preventSyncOfSameInstance"', () => {
const { result: resultA } = renderHook(() =>
useSharedState(identifier)
)
const { result: resultB } = renderHook(() =>
useSharedState(identifier)
)

expect(resultA.current.data).toEqual(undefined)
expect(resultB.current.data).toEqual(undefined)

act(() => {
resultA.current.update({ foo: 'bar' })
})

expect(resultA.current.data).toEqual({ foo: 'bar' })
expect(resultB.current.data).toEqual({ foo: 'bar' })

act(() => {
// If "preventSyncOfSameInstance" is set to true,
// then the "resultA" will not be synced, so resultA will still have "bar".
resultB.current.update(
{ foo: 'baz' },
{ preventSyncOfSameInstance: true }
)
})

expect(resultA.current.data).toEqual({ foo: 'bar' })
expect(resultB.current.data).toEqual({ foo: 'baz' })
})
})

describe('createReferenceKey', () => {
Expand Down
92 changes: 63 additions & 29 deletions packages/dnb-eufemia/src/shared/helpers/useSharedState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,24 @@ export function useSharedState<Data>(
onChange = null
) {
const [, forceUpdate] = useReducer(() => ({}), {})
const hasMounted = useMounted()
const hasMountedRef = useMounted()
const waitForMountedRef = useRef(false)
const instanceRef = useRef({})

const forceRerender = useCallback(() => {
if (hasMounted.current) {
if (hasMountedRef.current) {
forceUpdate()
} else {
waitForMountedRef.current = true
}
}, [hasMounted])
}, [hasMountedRef])

const shouldSync = useCallback((fn: () => void) => {
// Do not rerender the "same component", when the hook is used. Only other subscribers will rerender.
if (instanceRef.current === fn?.['ref']) {
return false
}
}, [])

useMountEffect(() => {
if (waitForMountedRef.current) {
Expand All @@ -50,16 +58,20 @@ export function useSharedState<Data>(

const sharedState = useMemo(() => {
if (id) {
return createSharedState<Data>(id, initialData)
return createSharedState<Data>(id, initialData, { shouldSync })
}
}, [id, initialData])
}, [id, initialData, shouldSync])
const sharedAttachment = useMemo(() => {
if (id) {
return createSharedState(createReferenceKey(id, 'oc'), { onChange })
return createSharedState(
createReferenceKey(id, 'oc'),
{ onChange },
{ shouldSync }
)
}
}, [id, onChange])
}, [id, onChange, shouldSync])

const sync = useCallback(
const syncAttachment = useCallback(
(newData: Data) => {
if (id) {
sharedAttachment.data?.onChange?.(newData)
Expand All @@ -69,39 +81,46 @@ export function useSharedState<Data>(
)

const update = useCallback(
(newData: Data) => {
(newData: Data, opts?: Options) => {
if (id) {
sharedState.update(newData)
sharedState.update(newData, opts)
}
},
[id, sharedState]
)

const get = useCallback(() => {
if (id) {
return sharedState?.get?.()
}
}, [id, sharedState])

const set = useCallback(
(newData: Data) => {
if (id) {
sharedState.set(newData)
sync(newData)
syncAttachment(newData)
}
},
[id, sharedState, sync]
[id, sharedState, syncAttachment]
)

const extend = useCallback(
(newData: Data) => {
(newData: Data, opts?: Options) => {
if (id) {
sharedState.extend(newData)
sync(newData)
sharedState.extend(newData, opts)
syncAttachment(newData)
}
},
[id, sharedState, sync]
[id, sharedState, syncAttachment]
)

useLayoutEffect(() => {
if (!id) {
return
}

forceRerender['ref'] = instanceRef.current
sharedState.subscribe(forceRerender)

return () => {
Expand All @@ -117,13 +136,12 @@ export function useSharedState<Data>(
}, [id, onChange, sharedAttachment])

return {
get: sharedState?.get,
get,
data: sharedState?.get?.() as Data,
hadInitialData: sharedState?.hadInitialData,
update,
set,
extend,
sync,
}
}

Expand All @@ -133,8 +151,8 @@ export interface SharedStateReturn<Data = undefined> {
data: Data
get: () => Data
set: (newData: Partial<Data>) => void
extend: (newData: Partial<Data>) => void
update: (newData: Partial<Data>) => void
extend: (newData: Partial<Data>, opts?: Options) => void
update: (newData: Partial<Data>, opts?: Options) => void
}

interface SharedStateInstance<Data> extends SharedStateReturn<Data> {
Expand All @@ -148,35 +166,55 @@ const sharedStates: Map<
SharedStateInstance<any>
> = new Map()

type Options = {
preventSyncOfSameInstance?: boolean
}

/**
* Creates a shared state instance with the specified ID and initial data.
*/
export function createSharedState<Data>(
/** The identifier for the shared state. */
id: SharedStateId,
/** The initial data for the shared state. */
initialData?: Data
initialData?: Data,
/** Optional configuration options. */
{
/** A function that returns true if the component should be rerendered. */
shouldSync = null,
} = {}
): SharedStateInstance<Data> {
if (!sharedStates.get(id)) {
let subscribers: Subscriber[] = []

const sync = (opts: Options = {}) => {
subscribers.forEach((subscriber) => {
const syncNow = opts.preventSyncOfSameInstance
? shouldSync?.(subscriber) !== false
: true
if (syncNow) {
subscriber()
}
})
}

const get = () => sharedStates.get(id).data

const set = (newData: Partial<Data>) => {
sharedStates.get(id).data = { ...newData }
}

const update = (newData: Partial<Data>) => {
const update = (newData: Partial<Data>, opts?: Options) => {
set(newData)
sync()
sync(opts)
}

const extend = (newData: Data) => {
const extend = (newData: Data, opts?: Options) => {
sharedStates.get(id).data = {
...sharedStates.get(id).data,
...newData,
}
sync()
sync(opts)
}

const subscribe = (subscriber: Subscriber) => {
Expand All @@ -189,10 +227,6 @@ export function createSharedState<Data>(
subscribers = subscribers.filter((sub) => sub !== subscriber)
}

const sync = () => {
subscribers.forEach((subscriber) => subscriber())
}

sharedStates.set(id, {
data: undefined,
get,
Expand Down

0 comments on commit 7de5e49

Please sign in to comment.