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

[Doc] Update the hooks documentation for consistent TOC #9453

Merged
merged 7 commits into from
Dec 19, 2023

Conversation

sebashwa
Copy link
Contributor

@sebashwa sebashwa commented Nov 16, 2023

Records returned from a context / controller hook are sometimes undefined or a cached version of the record. This is now stated more explicitly in the hook documentations for hooks that return the record from a context.

Things changed:

  • Had to move some documentation out of code blocks to add those links to the caching page.
  • Tried to make the documentation for the list / edit / create / show hooks more consistent (added headings / using the same documentation format for Arguments / Return Value).
  • Added "Props"-Headings for some more components to pop up in the navigation

Related to #9451

sebashwa added 2 commits November 17, 2023 00:35
The record returned by useRecordContext, useShowController, useShowContext,
useEditController and useEditContext is sometimes a cached version of
the record or undefined.

This is now stated more explicitly in the docs and a link to
the caching page is added.
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Although I don't agree with all your proposed changes.

Comment on lines 45 to 53
```jsx
const {
defaultTitle, // the translated title based on the resource, e.g. 'Create New Post'
redirect, // the default redirection route. Defaults to 'list'
resource, // the resource name, deduced from the location. e.g. 'posts'
save, // the update callback, to be passed to the underlying form as submit handler
saving, // boolean that becomes true when the dataProvider is called to create the record
} = useCreateContext();
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the previous version

Comment on lines 57 to 73
```jsx
const {
defaultTitle, // the translated title based on the resource, e.g. 'Create New Post'
redirect, // the default redirection route. Defaults to 'list'
resource, // the resource name, deduced from the location. e.g. 'posts'
save, // the update callback, to be passed to the underlying form as submit handler
saving, // boolean that becomes true when the dataProvider is called to create the record
} = useCreateController();
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this version

Comment on lines 48 to 64
The `useEditContext` hook returns an object with the same keys as returned by [`useEditController`](./useEditController.md):

```jsx
const {
defaultTitle, // the translated title based on the resource, e.g. 'Post #123'
error, // error returned by dataProvider when it failed to fetch the record. Useful if you want to adapt the view instead of just showing a notification using the `onError` side effect.
isFetching, // boolean that is true while the record is being fetched, and false once the record is fetched
isLoading, // boolean that is true until the record is available for the first time
mutationMode, // mutation mode argument passed as parameter, or 'undoable' if not defined
record, // record fetched via dataProvider.getOne() based on the id from the location
redirect, // the default redirection route. Defaults to 'list'
refetch, // a function that allows you to refetch the record
resource, // the resource name, deduced from the location. e.g. 'posts'
save, // the update callback, to be passed to the underlying form as submit handler
saving, // boolean that becomes true when the dataProvider is called to update the record
} = useEditContext();
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


`useEditController` returns an object with the following keys:

* `defaultTitle`: Translated title based on the resource, e.g. 'Post #123'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the object format


```jsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the object syntax

@@ -141,39 +141,46 @@ You can disable this feature by setting the `storeKey` prop to `false`. When dis

## Return Value

The return value of `useListController` has the following shape:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PLease keep the object format


`useSaveContext` returns an object with the following keys:

* `save`: Create or update callback which receives form data and calls `dataProvider`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAke it a code snippet


`useShowContext` returns an object with the same keys as [`useShowController`](./useShowController.md):

* `defaultTitle`: Translated title based on the resource, e.g. 'Post #123'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


`useShowController` returns an object with the following keys:

* `defaultTitle`: Translated title based on the resource, e.g. 'Post #123'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use an object in a code snippet

@@ -43,24 +43,24 @@ export const BookCreate = () => {

**Tip**: If you just use the return value of `useCreateController` to put it in an `CreateContext`, use [the `<CreateBase>` component](./CreateBase.md) instead for simpler markup.

`useCreateController` accepts an options argument, with the following fields, all optional:
## Arguments
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use Parameters in most hooks. Why is Arguments better? And if you change it here, you must change it everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure what's the preference, both work for me. Will change it back soon!

@sebashwa
Copy link
Contributor Author

sebashwa commented Nov 17, 2023

Hey @fzaninotto,

thanks for your feedback. My intention to use the listing instead of the object syntax in a codeblock were:

  • It's more consistent with the documentation for parameters (which are listings in most of the places, although sometimes also tables in case types / default values are documented). I don't see why the return value should be documented differently from the parameters.
  • It's not necessary to scroll to the right to read the descriptions of the object keys (at least I have to, maybe it's a resolution thing)
  • I thought it's not possible to put links inside codeblocks and I wanted to link to the "Caching" page for the returned record key. This was the main reason why I abandoned the codeblocks, although I think now, that's actually not true.

If you still think I should switch back to codeblocks, I will of course do so but imho I think it's neither more readable nor consistent with the rest of the documentation. What do you think?

@fzaninotto
Copy link
Member

I hear your arguments, yet I prefer the code snippet for the return values.

@fzaninotto
Copy link
Member

Any updates?

sebashwa added 2 commits December 5, 2023 22:20
Document hooks return values as code blocks with comments.
Remove links to Caching page with hints to caching page, as it's not
possible to link to other pages from within codeblocks.
@sebashwa
Copy link
Contributor Author

sebashwa commented Dec 5, 2023

Any updates?

Wasn't able to add those changes earlier.
I reverted to the code block documentation style, although it's now no longer possible to link to the Caching page from within the code block. So it just remains a hint to the Caching page for now.

@fzaninotto fzaninotto merged commit 8599362 into marmelab:master Dec 19, 2023
@fzaninotto fzaninotto added this to the 4.16.4 milestone Dec 19, 2023
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto changed the title Link to caching documentation page for hooks returning records [Doc] Update the hooks documentation for consistent TOC Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants