Add previous data on transform #7102
Conversation
docs/CreateEdit.md
Outdated
| @@ -5,7 +5,7 @@ title: "The Create and Edit Views" | |||
|
|
|||
| # The Create and Edit Views | |||
|
|
|||
| `<Resource>` maps URLs to components - it takes care of *routing*. When you set a component as the `create` prop for a Resource, react-admin renders that component when users go to the `/[resource]/create` URL. When you set a component as the `edit` prop for a resource, react-admin renders that component when users go to the `/[resource]/:id` URL. | |||
| `<Resource>` maps URLs to components - it takes care of *routing*. When you set a component as the `create` prop for a Resource, react-admin renders that component when users go to the `/[resource]/create` URL. When you set a component as the `edit` prop for a resource, react-admin renders that component when users go to the `/[resource]/:id` URL. | |||
There was a problem hiding this comment.
There are a lot of unrelated and unnecessary changes here due to your markdown formatter probably. Can you please revert them?
There was a problem hiding this comment.
Sure. Maybe my editor or prettier convert trailing space, I'll fix it asap.
docs/CreateEdit.md
Outdated
| **Tip**: `<Edit>`'s transform prop can recieve below variables from second argument. | ||
|
|
||
| | field | Description | | ||
| | -------------- | ------------------------------------------------------------------------------------------ | | ||
| | `previousData` | Previous data fetched or last updated on `dataProvider.update` | | ||
|
|
There was a problem hiding this comment.
| **Tip**: `<Edit>`'s transform prop can recieve below variables from second argument. | |
| | field | Description | | |
| | -------------- | ------------------------------------------------------------------------------------------ | | |
| | `previousData` | Previous data fetched or last updated on `dataProvider.update` | | |
| **Tip**: `<Edit>`'s transform prop function also get the `previousData` in its second argument: |
docs/CreateEdit.md
Outdated
| **Tip**: `<SaveButton>`'s transform prop inside `<Edit>` can recieve below variables from second argument. | ||
|
|
||
| | field | Description | | ||
| | -------------- | ------------------------------------------------------------------------------------------ | | ||
| | `previousData` | Previous data fetched or last updated on `dataProvider.update` | |
There was a problem hiding this comment.
| **Tip**: `<SaveButton>`'s transform prop inside `<Edit>` can recieve below variables from second argument. | |
| | field | Description | | |
| | -------------- | ------------------------------------------------------------------------------------------ | | |
| | `previousData` | Previous data fetched or last updated on `dataProvider.update` | | |
| **Tip**: `<Edit>`'s transform prop function also get the `previousData` in its second argument: |
tkow
force-pushed
the
feat/7054-previous-data-on-transform
branch
from
7b68bda
to
500b47b
Compare
tkow
force-pushed
the
feat/7054-previous-data-on-transform
branch
from
500b47b
to
ff3c330
Compare
|
@djhi
|
|
@djhi I'm sorry, but I couldn't find why Vercel - react-admin-storybook deployment has failed because I have no idea to know that without vercel logs. It can be build in my local environment. |
| @@ -116,9 +118,13 @@ export const useEditController = <RecordType extends RaRecord = any>( | |||
| ) => | |||
| Promise.resolve( | |||
| transformFromSave | |||
| ? transformFromSave(data) | |||
| ? transformFromSave(data, { | |||
There was a problem hiding this comment.
check the linter warning for missing dependency
There was a problem hiding this comment.
Thank you I've not noticed. It's green now.
|
|
||
| ```jsx | ||
| export const UserEdit = (props) => { | ||
| const transform = (data, { previousData }) => ({ |
There was a problem hiding this comment.
I'd prefer that the transform function signature be (data, previousData)
There was a problem hiding this comment.
Sorry, but I'd prefer my way, because it rarely breaks user code when we need to add more optionalData compared to push it to the tail of arguments though the change may be seldom.
Increasing arguments have risks when we want to add new argument, intermediate arguments can't change order without breaking change and even if they are not used in some cases, we may need to assign undefined like, (data, undefined, someEditControllerParameter) when we call or when we set no used variables' specification like, (data, _, someEditControllerParameter) as event hander.
Argument addition as option is better way for me if the argument is really optional value. In this case, users can ignore previousData so, it's optional value.
But, I can't say that previousData must be optional form because I haven't found the case we need more additional parameter in the future. My point is opposite to YAGNI, and previousData may be just enough parameter in all edit controller context parameters to satisfy user demands. So, I'll fix that if you still want to do nonetheless you have read this comment.
|
The react-admin-storybook deployment randomly fails, don't worry about It. I redeployed it manually. |
tkow
force-pushed
the
feat/7054-previous-data-on-transform
branch
from
889a264
to
b2895a0
Compare
tkow
force-pushed
the
feat/7054-previous-data-on-transform
branch
from
b2895a0
to
6216be2
Compare
|
I rebased my commits to remove no diff against next branch merged commits and fixing reviewed commits. |
|
Thanks! |
Closes #7054
Add previous data argument transform method. My tests added is redundant because previousData always is passed and asserted in every tests but, it's noting worth to keep description about expecting behavior, ready for this behavior is modified (however if you don't need it, I'll remove them).