-
-
Notifications
You must be signed in to change notification settings - Fork 352
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
CRUD component #4486
base: master
Are you sure you want to change the base?
CRUD component #4486
Conversation
Netlify deploy preview |
|
||
const { fields, createOne, validate } = cachedDataSource; | ||
|
||
const [formState, setFormState] = React.useState<{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formState calculation seems quite expensive to be made every render, here's a performance suggestion (including removing the .filter.map
chain and using a single .reduce
):
const createInitialValues = React.useCallback((fields, initialValues) => {
const baseValues = fields.reduce((acc, { field, type }) => {
if (field === 'id') return acc;
acc[field] = type === 'boolean' ? (initialValues[field] ?? false) : initialValues[field];
return acc;
}, {} as Partial<OmitId<D>>);
return {
values: { ...baseValues, ...initialValues },
errors: {} as Partial<Record<keyof D, string>>
};
}, []);
const [formState, setFormState] = React.useState(() =>
createInitialValues(fields, initialValues)
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that's only the initial state value, it doesn't run every render.
I think it should be ok, .reduce
might be more efficient but not as readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, i just looked it up and looks like it might run every render...
I guess that just passing a function with the same result still using .map
and .filter
would only run once though, I'll do that if it sounds cool. (just added it)
Implement CRUD feature as specified in #4146
https://deploy-preview-4486--mui-toolpad-docs.netlify.app/toolpad/core/react-crud/
Tests and examples will be added in separate PRs as this PR has already grown too large.
To help review, the contents of this PR are mostly:
packages/toolpad-core
.docs
, many examples but with a lot of repetition between them, feel free to just mostly take a look at the new documentation page for theCrud
component.playground
s for Vite, Next.js App Router and Next.js Pages Router.It took a while to reach this first version but still feel free to provide any major or minor feedback!