[BC Break] Add support mounting admin inside a sub path #7100

fzaninotto · 2022-01-17T16:46:21Z

Problems

React-router v6 doesn't support nesting routers. So it will break for users who use react-router to mount the admin in a sub path (e.g. "admin").

The way react-admin v3 used to support sub path relied on passing the basePath everywhere, which is not very good DX.

We need to reintroduce the ability to mount react-admin in a sub path, without using basePath.

Use case

const App = () => (
    <BrowserRouter>
        <Routes>
            <Route
                path="/admin/*"
                element={
                    <Admin dataProvider={dataProvider} basename="/admin">
                        <Resource name="posts" {...posts} />
                        <Resource name="comments" {...comments} />
                    </Admin>
                }
            />
        </Routes>
    </BrowserRouter>,
);

Solution

fzaninotto · 2022-01-17T16:50:08Z

packages/ra-core/src/routing/AdminRouter.tsx

+import { useInRouterContext } from 'react-router-dom';
+import { History } from 'history';
+
+import { HistoryRouter } from './HistoryRouter';


I upgraded react-router to 6.1 to get their new HistoryRouter, but they've made in "unstable" in v6.1.1, so we still need to use our own.

fzaninotto · 2022-01-17T16:51:28Z

packages/ra-core/package.json

@@ -38,8 +38,8 @@
        "react-dom": "^17.0.0",
        "react-final-form": "^6.5.7",
        "react-redux": "^7.1.0",
-        "react-router": "^6.0.2",


This change is not necessary - I wanted their HistoryRouter but the've hidden it behind an "unstable" flag.

As previously mentionned, there's v6.2.1 now: https://github.com/remix-run/react-router/releases

fzaninotto · 2022-01-17T16:52:00Z

docs/Fields.md

@@ -476,16 +476,16 @@ And now you can use a regular Field component, and the label displays correctly

 ### Linking To Other Records

-A custom Field component might need to display a link to another record. React Admin provides a `linkToRecord(basePath, id[, linkType])` method for this purpose.
+A custom Field component might need to display a link to another record. Build the URL to the distant record using the resource name and the id, as follows:


Probably a good time to introduce useCreateInternalLink

packages/ra-core/src/routing/useCreateInternalLink.ts

examples/crm/src/companies/CompanyCard.tsx

fzaninotto · 2022-01-17T17:03:35Z

packages/ra-core/src/util/index.ts

@@ -3,11 +3,9 @@ import FieldTitle, { FieldTitleProps } from './FieldTitle';
 import getFetchedAt from './getFetchedAt';
 import getFieldLabelTranslationArgs from './getFieldLabelTranslationArgs';
 import ComponentPropType from './ComponentPropType';
-import linkToRecord from './linkToRecord';


moved to the routing directory

packages/ra-ui-materialui/src/button/CreateButton.tsx

packages/ra-ui-materialui/src/field/ReferenceField.tsx

packages/ra-core/src/routing/useCreateInternalLink.ts

fzaninotto · 2022-01-17T17:08:36Z

packages/react-admin/src/AdminRouter.tsx

@@ -1,11 +0,0 @@
-import * as React from 'react';


useless component: LoadingPage is already set as default prop in AdminUI

wattroll · 2022-01-17T17:56:53Z

Looks great. I was setting up my own AdminContext to use BrowserRouter (passing history: createBrowserHistory() wasn't working for some reason). So apart from subpath mounting, this will bring a smooth way of changing the default router. Thanks.

Is history and HistoryRouter still needed per-se? From what I understood the main need for it came from using react-connected-router which isn't used anymore. Of course it's a question of BC breakage (as history is a prop of Admin), but it won't be big breakage if the fix is to just wrap Admin with HistoryRouter { history } or straight up with the preferred Router implementation.

Should the docs provide advice to switch away from HashRouter? I totally understand it being the default to facilitate getting started quickly, but e.g. react-router docs say "We strongly recommend you do not use HashRouter unless you absolutely have to.".

djhi

Neat! This is so much better 💪

UPGRADE.md

djhi · 2022-01-19T09:02:51Z

examples/crm/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0",


v6.2.1 is out: https://github.com/remix-run/react-router/releases

djhi · 2022-01-19T09:03:54Z

examples/demo/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0",


Same: https://github.com/remix-run/react-router/releases

djhi · 2022-01-19T09:04:42Z

examples/simple/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0"


Same: https://github.com/remix-run/react-router/releases

djhi · 2022-01-19T09:05:33Z

packages/ra-core/package.json

@@ -38,8 +38,8 @@
        "react-dom": "^17.0.0",
        "react-final-form": "^6.5.7",
        "react-redux": "^7.1.0",
-        "react-router": "^6.0.2",


As previously mentionned, there's v6.2.1 now: https://github.com/remix-run/react-router/releases

djhi · 2022-01-19T09:08:51Z

packages/ra-core/src/routing/linkToRecord.ts

+/**
+ * @deprecated use useCreatePath instead
+ */
+export const linkToRecord = (resource, id, linkType = 'edit') => {
+    const link = `${resource}/${encodeURIComponent(id)}`;
+
+    if (linkType === 'show') {
+        return `${link}/show`;
+    }
+
+    return link;
+};


Should be deprecated in next v3 and removed here

We don't need to remove it here - it would force an even longer upgrade.

It's just not used anymore, and doesn't do any harm until v5.

djhi · 2022-01-19T09:09:20Z

packages/ra-core/src/routing/resolveRedirectTo.ts

+import { To } from 'react-router-dom';
+
+/**
+ * @deprecated use useCreatePath instead


Same answer: we don't remove it to avoid a more painful upgrade.

djhi · 2022-01-19T09:10:17Z

packages/ra-no-code/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0",


Same, more recent version available

djhi · 2022-01-19T09:10:29Z

packages/ra-test/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0",


djhi · 2022-01-19T09:10:38Z

packages/ra-ui-materialui/package.json

+        "react-router": "^6.1.0",
+        "react-router-dom": "^6.1.0",


UPGRADE.md

WiXSL

Impressive work!
I couldn't been able to find problems 💪

fzaninotto · 2022-01-19T14:40:33Z

@djhi Let's upgrade the packages to their latest version in another PR (and rather at the end of the 4.0 roadmap), to avoid multiple rebases due to yarn.lock conflicts.

fzaninotto · 2022-01-19T14:43:42Z

@wattroll

Is history and HistoryRouter still needed per-se? From what I understood the main need for it came from using react-connected-router which isn't used anymore. Of course it's a question of BC breakage (as history is a prop of Admin), but it won't be big breakage if the fix is to just wrap Admin with HistoryRouter { history } or straight up with the preferred Router implementation.

No, <Admin history> is no longer necessary, but we don't need to remove it. I've marked it as deprecated.

Should the docs provide advice to switch away from HashRouter? I totally understand it being the default to facilitate getting started quickly, but e.g. react-router docs say "We strongly recommend you do not use HashRouter unless you absolutely have to.".

I added a section about it, see Routing.md

fzaninotto added the WIP Work In Progress label Jan 17, 2022

fzaninotto added this to the 4.0.0-alpha.2 milestone Jan 17, 2022

fzaninotto mentioned this pull request Jan 17, 2022

4.0 Roadmap #5933

Closed

fzaninotto changed the title ~~Add support mounting admin unside a sub path~~ [BC Break] Add support mounting admin unside a sub path Jan 17, 2022

vercel bot temporarily deployed to Preview – react-admin January 17, 2022 16:55 Inactive

vercel bot deployed to Preview – react-admin-storybook January 17, 2022 16:55 View deployment

vercel bot temporarily deployed to Preview – react-admin January 17, 2022 17:00 Inactive

vercel bot deployed to Preview – react-admin-storybook January 17, 2022 17:05 View deployment

fzaninotto commented Jan 17, 2022

View reviewed changes

fzaninotto added 7 commits January 17, 2022 18:17

Initial commit

4c9bfb4

Support basename in more components

5d87dec

Migrate more components

394bdab

Fix tebbedForm routing

e40de47

Fix linter warnings

05ce158

Fix TS compilation

fc10252

Add yarn.lock

d825395

fzaninotto force-pushed the basename branch from 4a44a0e to d825395 Compare January 17, 2022 17:20

vercel bot temporarily deployed to Preview – react-admin January 17, 2022 17:20 Inactive

vercel bot deployed to Preview – react-admin-storybook January 17, 2022 17:20 View deployment

fzaninotto added 2 commits January 17, 2022 18:24

Fix linter warning

32b3ac6

Fix redirection side effects

523072b

vercel bot temporarily deployed to Preview – react-admin January 17, 2022 17:34 Inactive

vercel bot had a problem deploying to Preview – react-admin-storybook January 17, 2022 17:34 Failure

fzaninotto changed the title ~~[BC Break] Add support mounting admin unside a sub path~~ [BC Break] Add support mounting admin inside a sub path Jan 17, 2022

fzaninotto added 4 commits January 17, 2022 19:00

Remove basepath on Datagrid

6a4ce23

Remove now useless basePath

a1c14db

Fix CRM example

972d758

Remove basePath mentions in documentation

5fe5191

vercel bot temporarily deployed to Preview – react-admin January 17, 2022 19:53 Inactive

vercel bot had a problem deploying to Preview – react-admin-storybook January 18, 2022 19:14 Failure

Add tests for mounting admin inside another app

3c0761b

vercel bot temporarily deployed to Preview – react-admin January 18, 2022 19:25 Inactive

vercel bot deployed to Preview – react-admin-storybook January 18, 2022 19:25 View deployment

fzaninotto added 8 commits January 18, 2022 23:50

Fix linter wqrning

7d3d0cb

Add more tests for useCreatePath

7fce20d

Fix routing doc

f459e05

Fix linter warning

c1d762e

Avoids rerender in buttons

b9e2073

Fix wrong type

351fea0

Fix linter warning

93776b2

Add upgrade guide

d103263

vercel bot temporarily deployed to Preview – react-admin January 18, 2022 23:29 Inactive

vercel bot had a problem deploying to Preview – react-admin-storybook January 18, 2022 23:29 Failure

fzaninotto added RFR Ready For Review and removed WIP Work In Progress labels Jan 18, 2022

vercel bot deployed to Preview – react-admin-storybook January 18, 2022 23:31 View deployment

djhi requested changes Jan 19, 2022

View reviewed changes

WiXSL suggested changes Jan 19, 2022

View reviewed changes

UPGRADE.md Outdated Show resolved Hide resolved

WiXSL reviewed Jan 19, 2022

View reviewed changes

WiXSL approved these changes Jan 19, 2022

View reviewed changes

Review

c7672f4

vercel bot had a problem deploying to Preview – react-admin-storybook January 19, 2022 14:39 Failure

vercel bot temporarily deployed to Preview – react-admin January 19, 2022 14:39 Inactive

vercel bot deployed to Preview – react-admin-storybook January 19, 2022 14:41 View deployment

djhi approved these changes Jan 19, 2022

View reviewed changes

djhi merged commit 6814f34 into next Jan 19, 2022

djhi deleted the basename branch January 19, 2022 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BC Break] Add support mounting admin inside a sub path #7100

[BC Break] Add support mounting admin inside a sub path #7100

fzaninotto commented Jan 17, 2022 •

edited

Loading

fzaninotto Jan 17, 2022

fzaninotto Jan 17, 2022

djhi Jan 19, 2022

fzaninotto Jan 17, 2022 •

edited by WiXSL

Loading

fzaninotto Jan 17, 2022

fzaninotto Jan 17, 2022

wattroll commented Jan 17, 2022 •

edited

Loading

djhi left a comment

djhi Jan 19, 2022

djhi Jan 19, 2022

djhi Jan 19, 2022

djhi Jan 19, 2022

djhi Jan 19, 2022

fzaninotto Jan 19, 2022

djhi Jan 19, 2022

fzaninotto Jan 19, 2022

djhi Jan 19, 2022

djhi Jan 19, 2022

djhi Jan 19, 2022

WiXSL left a comment •

edited

Loading

fzaninotto commented Jan 19, 2022

fzaninotto commented Jan 19, 2022

[BC Break] Add support mounting admin inside a sub path #7100

[BC Break] Add support mounting admin inside a sub path #7100

Conversation

fzaninotto commented Jan 17, 2022 • edited Loading

Problems

Use case

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzaninotto Jan 17, 2022 • edited by WiXSL Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wattroll commented Jan 17, 2022 • edited Loading

djhi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WiXSL left a comment • edited Loading

Choose a reason for hiding this comment

fzaninotto commented Jan 19, 2022

fzaninotto commented Jan 19, 2022

fzaninotto commented Jan 17, 2022 •

edited

Loading

fzaninotto Jan 17, 2022 •

edited by WiXSL

Loading

wattroll commented Jan 17, 2022 •

edited

Loading

WiXSL left a comment •

edited

Loading