-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Remove permissions prop injection from main views #6921
Conversation
}; | ||
|
||
export type UseAuthenticatedOptions<ParamsType> = { | ||
enabled?: boolean; |
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.
It would be great to have documentation of the new enabled
prop
@@ -40,7 +40,7 @@ import useNotify from '../sideEffect/useNotify'; | |||
* return authenticated ? <Bar /> : <BarNotAuthenticated />; | |||
* } // tip: use useAuthState() hook instead | |||
*/ | |||
const useCheckAuth = (): CheckAuth => { | |||
export const useCheckAuth = (): CheckAuth => { |
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.
Missing export for type CheckAuth
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 Resource component is becoming smaller and smaller... If we ever remove the registration phase, it'll just be routes!
@@ -28,9 +26,21 @@ const emptyParams = {}; | |||
* </Admin> | |||
* ); | |||
*/ | |||
export default (params: any = emptyParams) => { | |||
export const useAuthenticated = <ParamsType = any>( |
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.
this is a breaking change for users who used to pass params, and should be mentioned in the upgrade guide
pathname: '/foo', | ||
search: undefined, | ||
state: undefined, | ||
hash: undefined, | ||
}; | ||
|
||
it('should return an empty record by default', () => { |
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.
why did you remove this one ?
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.
Because that's not the responsability of this function anymore
@@ -49,20 +51,23 @@ export const useCreateController = < | |||
props: CreateControllerProps = {} | |||
): CreateControllerResult<RecordType> => { | |||
const { | |||
record = {}, | |||
successMessage, | |||
disableAuthentication, |
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.
You should update the Auth doc to mention this prop. Same for other controllers.
Co-authored-by: Aníbal Svarcas <[email protected]>
This pull request is being automatically deployed with Vercel (learn more). react-admin – ./examples/simple🔍 Inspect: https://vercel.com/marmelab/react-admin/Dr6KUPTzF9ajoCepS7Q8rfBk9RWR react-admin-storybook – ./🔍 Inspect: https://vercel.com/marmelab/react-admin-storybook/CBquMNdhvCVpGTCMEHSUzHbXRZ33 |
No description provided.