[RFC] Rename AppProvider export for Next.js and react-router integrations

[garryxiao]

What's the problem?

I followed the example and found the example fails (keep refresh the whole page when click the nested links) when import the "AppProvider" with

import { AppProvider } from "@toolpad/core/AppProvider";

but works when use

import { AppProvider } from "@toolpad/core/react-router-dom"'

It's very confusing with the same name export but actual different. And not type 'Navigation' found of the 'core-vite' example line:

import { AppProvider, type Navigation } from '@toolpad/core/react-router-dom';

What are the requirements?

No response

What are our options?

Rename the export name and mention it in the documentation would be perfect.

Proposed solution

No response

Resources and benchmarks

No response

Search keywords:

[Janpot]

thank you, can you add a proposal for the new names under the options section?

When we reach consensus, would you be interested in contributing this change?

[apedroferreira]

One option that I considered before and thought about bringing up for consideration again was to export a hook to create the router with React Router, to be passed as a prop to the standard AppProvider, instead of having a specific AppProvider for React Router.
This requires creating an extra component sometimes because the hook requires the React Router context around it, but when using createBrowserRouter instead of BrowserRouter a separate component with the AppProvider is already needed anyway, and this approach probably makes more sense.

Besides that or a possible rename, we can also consider exporting all the types from the standard AppProvider in the other AppProvider alternatives.

[garryxiao]

My idea is to add a type declaration file called "AppProviderTypes.ts", export it globally and remove the "AppProvider" export at the same time, but the changes are huge. Or add "AppProviderComponent.ts", keep the types in "AppProvider.ts", that would be a better idea.

[Janpot]

I think we eventually want to be able to do more in the next.js app provider than just routing. A hook may be a bit limited in that sense?

I'm not sure it's a good idea to export the same interfaces from multiple places (barrel filles excluded). We don't do that anywhere, it could become confusing.

I think renaming it to something like AppRouterAppProvider makes quite a lot of sense, it would be analog to the AppRouterCacheProvider we have in @mui/material-nextjs.

[apedroferreira]

I think we eventually want to be able to do more in the next.js app provider than just routing. A hook may be a but limited in that sense?

For the Next.js AppProvider that's probably true, but for the React Router integration I wasn't so sure.
But it's just something we could consider and discuss later. I guess that supporting different types of app with different AppProviders might be more consistent overall too.

I think renaming it to something like AppRouterAppProvider makes quite a lot of sense, it would be analog to the AppRouterCacheProvider we have in @mui/material-nextjs.

Ok, I was thinking about something like ReactRouterAppProvider for the React Router integration so we could follow that pattern?

I'm not sure it's a good idea to export the same interfaces from multiple places (barrel filles excluded). We don't do that anywhere, it could become confusing.

Ok, we can keep types as they are for now, the renaming would already help reduce the confusion.

[EstherPerelmanCommit]

we lately created a new project with the toolpad and mistakenly imported the AppProvider from the wrong url, this was vaery hard to find out! each time we navigated between routes in the sidebar the app reloaded instead smoothly navigation

[apedroferreira]

Note: make sure we also solve the confusion in #4513 with this.