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

Fix logout doesn't reset resource registration #7539

Merged
merged 12 commits into from
Apr 27, 2022
5 changes: 4 additions & 1 deletion docs/Admin.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ import {
localStorageStore,
Resource,
ListGuesser,
Loading,
useDataProvider,
} from 'react-admin';

Expand All @@ -523,11 +524,13 @@ function AsyncResources() {
}, []);

return (
<AdminUI>
<AdminUI ready={Loading}>
{resources.map(resource => (
<Resource name={resource.name} key={resource.key} list={ListGuesser} />
))}
</AdminUI>
);
}
```

In this example, we override the `<AdminUI ready>` component to prevent the admin from displaying [the ready screen](#ready) in development while the list of resources is empty.
2 changes: 1 addition & 1 deletion packages/ra-core/src/auth/useLogout.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { useCallback, useEffect, useRef } from 'react';
import { useLocation, useNavigate, Path } from 'react-router-dom';

import useAuthProvider, { defaultAuthParams } from './useAuthProvider';
import { useResetStore } from '../store';
import { useBasename } from '../routing';
import { removeDoubleSlashes } from '../routing/useCreatePath';
import { useLocation, useNavigate, Path } from 'react-router-dom';

/**
* Get a callback for calling the authProvider.logout() method,
Expand Down
2 changes: 2 additions & 0 deletions packages/ra-core/src/core/CoreAdmin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export const CoreAdmin = (props: CoreAdminProps) => {
loading,
loginPage,
menu, // deprecated, use a custom layout instead
ready,
requireAuth,
title = 'React Admin',
} = props;
Expand All @@ -120,6 +121,7 @@ export const CoreAdmin = (props: CoreAdminProps) => {
loading={loading}
loginPage={loginPage}
requireAuth={requireAuth}
ready={ready}
>
{children}
</CoreAdminUI>
Expand Down
35 changes: 25 additions & 10 deletions packages/ra-core/src/core/ResourceDefinitionContext.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { createContext, useCallback, useState } from 'react';
import { createContext, useCallback, useState, useMemo } from 'react';
import isEqual from 'lodash/isEqual';

import { ResourceDefinition } from '../types';
Expand All @@ -8,14 +8,19 @@ export type ResourceDefinitions = {
[name: string]: ResourceDefinition;
};

export type ResourceDefinitionContextValue = [
ResourceDefinitions,
(config: ResourceDefinition) => void
];
export type ResourceDefinitionContextValue = {
definitions: ResourceDefinitions;
register: (config: ResourceDefinition) => void;
unregister: (config: ResourceDefinition) => void;
};

export const ResourceDefinitionContext = createContext<
ResourceDefinitionContextValue
>([{}, () => {}]);
>({
definitions: {},
register: () => {},
unregister: () => {},
});

/**
* Context to store the current resource Definition.
Expand Down Expand Up @@ -45,7 +50,7 @@ export const ResourceDefinitionContextProvider = ({
defaultDefinitions
);

const setDefinition = useCallback((config: ResourceDefinition) => {
const register = useCallback((config: ResourceDefinition) => {
setState(prev =>
isEqual(prev[config.name], config)
? prev
Expand All @@ -56,10 +61,20 @@ export const ResourceDefinitionContextProvider = ({
);
}, []);

const unregister = useCallback((config: ResourceDefinition) => {
setState(prev => {
const { [config.name]: _, ...rest } = prev;
return rest;
});
}, []);

const contextValue = useMemo(
() => ({ definitions, register, unregister }),
[definitions] // eslint-disable-line react-hooks/exhaustive-deps
);

return (
<ResourceDefinitionContext.Provider
value={[definitions, setDefinition]}
>
<ResourceDefinitionContext.Provider value={contextValue}>
{children}
</ResourceDefinitionContext.Provider>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-core/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export * from './ResourceContext';
export * from './ResourceContextProvider';
export * from './ResourceDefinitionContext';
export * from './useGetResourceLabel';
export * from './useRegisterResource';
export * from './useResourceDefinitionContext';
export * from './useResourceContext';
export * from './useResourceDefinition';
export * from './useResourceDefinitions';
104 changes: 104 additions & 0 deletions packages/ra-core/src/core/useConfigureAdminRouterFromChildren.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import * as React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import expect from 'expect';
import { createMemoryHistory } from 'history';
import { useResourceDefinitions } from './useResourceDefinitions';
import { CoreAdminContext } from './CoreAdminContext';
import { CoreAdminRoutes } from './CoreAdminRoutes';
import { Resource } from './Resource';
import { CoreLayoutProps } from '../types';

const ResourceDefinitionsTestComponent = () => {
const definitions = useResourceDefinitions();
if (!definitions) return null;
return (
<ul>
{Object.keys(definitions).map(key => (
<li key={key}>{JSON.stringify(definitions[key])}</li>
))}
</ul>
);
};

const MyLayout = ({ children }: CoreLayoutProps) => (
<>
<ResourceDefinitionsTestComponent />
{children}
</>
);
const CatchAll = () => <div />;
const Loading = () => <>Loading</>;

const TestedComponent = ({ role }) => {
const history = createMemoryHistory();

return (
<CoreAdminContext history={history}>
<CoreAdminRoutes
layout={MyLayout}
catchAll={CatchAll}
loading={Loading}
>
<Resource name="posts" />
<Resource name="comments" />
{() =>
role === 'admin'
? [<Resource name="user" />, <Resource name="admin" />]
: role === 'user'
? [<Resource name="user" />]
: []
}
</CoreAdminRoutes>
</CoreAdminContext>
);
};

const expectResource = (resource: string) =>
expect(screen.queryByText(`"name":"${resource}"`, { exact: false }));

describe('useConfigureAdminRouterFromChildren', () => {
it('should always load static resources', async () => {
render(<TestedComponent role="guest" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('posts').not.toBeNull();
expectResource('comments').not.toBeNull();
expectResource('user').toBeNull();
expectResource('admin').toBeNull();
});
it('should load dynamic resource definitions', async () => {
render(<TestedComponent role="admin" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('user').not.toBeNull();
expectResource('admin').not.toBeNull();
});
it('should allow adding new resource after the first render', async () => {
const { rerender } = render(<TestedComponent role="user" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('posts').not.toBeNull();
expectResource('comments').not.toBeNull();
expectResource('user').not.toBeNull();
expectResource('admin').toBeNull();

rerender(<TestedComponent role="admin" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('posts').not.toBeNull();
expectResource('comments').not.toBeNull();
expectResource('user').not.toBeNull();
expectResource('admin').not.toBeNull();
});
it('should allow removing a resource after the first render', async () => {
const { rerender } = render(<TestedComponent role="admin" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('posts').not.toBeNull();
expectResource('comments').not.toBeNull();
expectResource('user').not.toBeNull();
expectResource('admin').not.toBeNull();

rerender(<TestedComponent role="user" />);
await waitFor(() => expect(screen.queryByText('Loading')).toBeNull());
expectResource('posts').not.toBeNull();
expectResource('comments').not.toBeNull();
expectResource('user').not.toBeNull();
expectResource('admin').toBeNull();
});
});
25 changes: 21 additions & 4 deletions packages/ra-core/src/core/useConfigureAdminRouterFromChildren.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
ResourceProps,
} from '../types';
import { CustomRoutesProps } from './CustomRoutes';
import { useRegisterResource } from './useRegisterResource';
import { useResourceDefinitionContext } from './useResourceDefinitionContext';

/**
* This hook inspects the CoreAdminRouter children and returns them separated in three groups:
Expand All @@ -41,7 +41,7 @@ export const useConfigureAdminRouterFromChildren = (
const getPermissions = useGetPermissions();
const doLogout = useLogout();
const { authenticated } = useAuthState();
const registerResource = useRegisterResource();
const { register, unregister } = useResourceDefinitionContext();
// Gather custom routes and resources that were declared as direct children of CoreAdminRouter
// e.g. Not returned from the child function (if any)
// We need to know right away wether some resources were declared to correctly
Expand Down Expand Up @@ -155,14 +155,31 @@ export const useConfigureAdminRouterFromChildren = (
const definition = ((resource.type as unknown) as ResourceWithRegisterFunction).registerResource(
resource.props
);
registerResource(definition);
register(definition);
} else {
throw new Error(
'When using a custom Resource element, it must have a static registerResource method accepting its props and returning a ResourceDefinition'
);
}
});
}, [registerResource, resources]);
return () => {
resources.forEach(resource => {
if (
typeof ((resource.type as unknown) as ResourceWithRegisterFunction)
.registerResource === 'function'
) {
const definition = ((resource.type as unknown) as ResourceWithRegisterFunction).registerResource(
resource.props
);
unregister(definition);
} else {
throw new Error(
'When using a custom Resource element, it must have a static registerResource method accepting its props and returning a ResourceDefinition'
);
}
});
};
}, [register, unregister, resources]);

return {
customRoutesWithLayout,
Expand Down
19 changes: 0 additions & 19 deletions packages/ra-core/src/core/useRegisterResource.ts

This file was deleted.

6 changes: 6 additions & 0 deletions packages/ra-core/src/core/useResourceDefinitionContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { useContext } from 'react';

import { ResourceDefinitionContext } from './ResourceDefinitionContext';

export const useResourceDefinitionContext = () =>
useContext(ResourceDefinitionContext);
10 changes: 3 additions & 7 deletions packages/ra-core/src/core/useResourceDefinitions.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { useContext } from 'react';

import {
ResourceDefinitionContext,
ResourceDefinitions,
} from './ResourceDefinitionContext';
import { ResourceDefinitions } from './ResourceDefinitionContext';
import { useResourceDefinitionContext } from './useResourceDefinitionContext';

/**
* Get the definition of the all resources
Expand All @@ -23,4 +19,4 @@ import {
* // }
*/
export const useResourceDefinitions = (): ResourceDefinitions =>
useContext(ResourceDefinitionContext)[0];
useResourceDefinitionContext().definitions;