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

[Form lib] Memoize form hook object and fix hook array deps #71237

Merged
merged 17 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import React, { useEffect, useCallback, createContext, useContext } from 'react';
import React, { useEffect, useCallback, createContext, useContext, useRef } from 'react';

import { useMultiContent, HookProps, Content, MultiContent } from './use_multi_content';

Expand Down Expand Up @@ -55,7 +55,14 @@ export function useMultiContentContext<T extends object = { [key: string]: any }
* @param contentId The content id to be added to the "contents" map
*/
export function useContent<T extends object, K extends keyof T>(contentId: K) {
const { updateContentAt, saveSnapshotAndRemoveContent, getData } = useMultiContentContext<T>();
const isMounted = useRef(false);
const defaultValue = useRef<T[K] | undefined>(undefined);
const {
updateContentAt,
saveSnapshotAndRemoveContent,
getData,
getSingleContentData,
} = useMultiContentContext<T>();

const updateContent = useCallback(
(content: Content) => {
Expand All @@ -71,12 +78,22 @@ export function useContent<T extends object, K extends keyof T>(contentId: K) {
};
}, [contentId, saveSnapshotAndRemoveContent]);

const data = getData();
const defaultValue = data[contentId];
useEffect(() => {
if (isMounted.current === false) {
isMounted.current = true;
}
}, []);

if (isMounted.current === false) {
// Only read the default value once, on component mount to avoid re-rendering the
// consumer each time the multi-content validity ("isValid") changes.
defaultValue.current = getSingleContentData(contentId);
}

return {
defaultValue,
defaultValue: defaultValue.current!,
updateContent,
getData,
getSingleContentData,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export interface MultiContent<T extends object> {
updateContentAt: (id: keyof T, content: Content) => void;
saveSnapshotAndRemoveContent: (id: keyof T) => void;
getData: () => T;
getSingleContentData: <K extends keyof T>(contentId: K) => T[K];
validate: () => Promise<boolean>;
validation: Validation<T>;
}
Expand Down Expand Up @@ -109,9 +110,22 @@ export function useMultiContent<T extends object>({
};
}, [stateData, validation]);

/**
* Read a single content data.
*/
const getSingleContentData = useCallback(
<K extends keyof T>(contentId: K): T[K] => {
if (contents.current[contentId]) {
return contents.current[contentId].getData();
}
return stateData[contentId];
},
[stateData]
);

const updateContentValidity = useCallback(
(updatedData: { [key in keyof T]?: boolean | undefined }): boolean | undefined => {
let allContentValidity: boolean | undefined;
let isAllContentValid: boolean | undefined = validation.isValid;

setValidation((prev) => {
if (
Expand All @@ -120,7 +134,7 @@ export function useMultiContent<T extends object>({
)
) {
// No change in validation, nothing to update
allContentValidity = prev.isValid;
isAllContentValid = prev.isValid;
return prev;
}

Expand All @@ -129,21 +143,21 @@ export function useMultiContent<T extends object>({
...updatedData,
};

allContentValidity = Object.values(nextContentsValidityState).some(
isAllContentValid = Object.values(nextContentsValidityState).some(
(_isValid) => _isValid === undefined
)
? undefined
: Object.values(nextContentsValidityState).every(Boolean);

return {
isValid: allContentValidity,
isValid: isAllContentValid,
contents: nextContentsValidityState,
};
});

return allContentValidity;
return isAllContentValid;
},
[]
[validation.isValid]
);

/**
Expand All @@ -163,7 +177,7 @@ export function useMultiContent<T extends object>({
}

return Boolean(updateContentValidity(updatedValidation));
}, [updateContentValidity]);
}, [validation.isValid, updateContentValidity]);

/**
* Update a content. It replaces the content in our "contents" map and update
Expand All @@ -186,7 +200,7 @@ export function useMultiContent<T extends object>({
});
}
},
[updateContentValidity, onChange]
[updateContentValidity, onChange, getData, validate]
);

/**
Expand All @@ -211,6 +225,7 @@ export function useMultiContent<T extends object>({

return {
getData,
getSingleContentData,
validate,
validation,
updateContentAt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ interface Props {

export const FormDataProvider = React.memo(({ children, pathsToWatch }: Props) => {
const form = useFormContext();
const { subscribe } = form;
const previousRawData = useRef<FormData>(form.__getFormData$().value);
const [formData, setFormData] = useState<FormData>(previousRawData.current);

Expand All @@ -54,9 +55,9 @@ export const FormDataProvider = React.memo(({ children, pathsToWatch }: Props) =
);

useEffect(() => {
const subscription = form.subscribe(onFormData);
const subscription = subscribe(onFormData);
return subscription.unsubscribe;
}, [form.subscribe, onFormData]);
}, [subscribe, onFormData]);

return children(formData);
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { useState, useEffect, useRef } from 'react';
import { useState, useEffect, useRef, useCallback } from 'react';

import { useFormContext } from '../form_context';

Expand Down Expand Up @@ -83,14 +83,18 @@ export const UseArray = ({

const [items, setItems] = useState<ArrayItem[]>(initialState);

const updatePaths = (_rows: ArrayItem[]) =>
_rows.map(
(row, index) =>
({
...row,
path: `${path}[${index}]`,
} as ArrayItem)
);
const updatePaths = useCallback(
(_rows: ArrayItem[]) => {
return _rows.map(
(row, index) =>
({
...row,
path: `${path}[${index}]`,
} as ArrayItem)
);
},
[path]
);

const addItem = () => {
setItems((previousItems) => {
Expand All @@ -108,11 +112,13 @@ export const UseArray = ({

useEffect(() => {
if (didMountRef.current) {
setItems(updatePaths(items));
setItems((prev) => {
return updatePaths(prev);
});
} else {
didMountRef.current = true;
}
}, [path]);
}, [path, updatePaths]);

return children({ items, addItem, removeItem });
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ describe('<UseField />', () => {

const TestComp = ({ onData }: { onData: OnUpdateHandler }) => {
const { form } = useForm();
const { subscribe } = form;

useEffect(() => form.subscribe(onData).unsubscribe, [form]);
useEffect(() => subscribe(onData).unsubscribe, [subscribe, onData]);

return (
<Form form={form}>
Expand Down
Loading