Fix no-code codesandbox infinite loop [RFR]

[smeng9]

Before: https://codesandbox.io/s/github/marmelab/react-admin/tree/next/examples/no-code After: https://codesandbox.io/s/github/smeng9/react-admin/tree/nocodeexample/examples/no-code

Close https://github.com/marmelab/react-admin/issues/7453

~~Also updates dependencies to react 18~~ ~~Also remove deprecated @vitejs/plugin-react-refresh~~ ~~For the examples are you gradually moving to vite as the build tool than default webpack from create-react-app?~~

[smeng9]

No idea what is going on with the unit tests.

For // FIXME MUI bug https://github.com/mui-org/material-ui/issues/13394 seems it no longer throws out a warning

[slax57]

Hi @smeng9 ! Thank you very much for submitting this! 🙂

Your branch seems to run fine on my local env, so this is a good point.

Unfortunately, I haven't been able to get past the "Create new application" screen while in the codesandbox. After I click on my app's name I get a white screen. Does it behave the same way for you?

[smeng9]

@slax57 I believe so. It should also give you white screen locally.

I think it is a separate issue and we should open an additional ticket. It seems originate from the ra-no-code package itself. This pull request can be merged first because it already solves the rerender loop issue.

The error log from console is

Uncaught Error: useNavigate() may be used only in the context of a <Router> component.
    at invariant (index.tsx:19:20)
    at useNavigate (index.tsx:507:3)
    at ImportResourceDialog (ImportResourceDialog.tsx:22:22)
    at renderWithHooks (react-dom.development.js:16141:18)
    at mountIndeterminateComponent (react-dom.development.js:20838:13)
    at beginWork (react-dom.development.js:22342:16)
    at HTMLUnknownElement.callCallback2 (react-dom.development.js:4157:14)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:4206:16)
    at invokeGuardedCallback (react-dom.development.js:4270:31)
    at beginWork$1 (react-dom.development.js:27243:7)

at ImportResourceDialog (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/ra-no-code.js?v=96129ce9:3383:44)

    at Ready

    at CoreAdminRoutes (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:19763:12)

    at Routes (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:13419:15)

    at CoreAdminUI (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:19772:19)

    at AdminUI (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:51172:31)

    at InnerThemeProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-GJTOIRTA.js?v=96129ce9:6668:17)

    at ThemeProvider2 (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-GJTOIRTA.js?v=96129ce9:6407:5)

    at ThemeProvider3 (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-GJTOIRTA.js?v=96129ce9:6676:5)

    at ThemeProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:49529:27)

    at ResourceDefinitionContextProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:19686:28)

    at NotificationContextProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18829:25)

    at I18nContextProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18840:22)

    at Router (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:13366:15)

    at HistoryRouter (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18600:25)

    at BasenameContextProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18606:25)

    at AdminRouter (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18607:24)

    at QueryClientProvider2 (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:12429:21)

    at StoreContextProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:18804:22)

    at CoreAdminContext (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:19693:23)

    at AdminContext (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:51178:24)

    at Admin (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/chunk-DRS7MMIR.js?v=96129ce9:51226:23)

    at InnerAdmin (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/ra-no-code.js?v=96129ce9:3449:13)

    at ResourceConfigurationProvider (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/ra-no-code.js?v=96129ce9:3212:25)

    at Admin (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/ra-no-code.js?v=96129ce9:3442:28)

    at Root (https://t8yxse.sse.codesandbox.io/node_modules/.vite/deps/ra-no-code.js?v=96129ce9:3487:22)


Consider adding an error boundary to your tree to customize error handling behavior.

Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.

[smeng9]

Hi @slax57

The reason is there are missing peer dependencies so two instance of react-router is created. Can you confirm that?

[slax57]

Hi @smeng9 Sorry for the delay of my answer.

I've taken some time to look into this, but actually it's a bit denser than what I originally thought, at least for me 😅 .

Concerning your PR:

• I'm not sure it's a good idea to include an upgrade to React 18 yet. Indeed there may be side effects or incompatibilities that we still need to test out a bit before updating (see https://github.com/marmelab/react-admin/issues/7494 for instance 😅 ), plus I'd rather have a PR address 1 thing at a time
• Event if it fixes the flashing problem, I don't think we should merge it yet because the codesandbox is still unusable afterwards. I think we should rather fix the codesandbox for good, rather than letting people think it will work although it won't.

Concerning the actual issue:

• The key fix is indeed adding the hmr section to vite.config.ts
• The problem occurs on the codesandbox because of the resolve aliases being absent in vite.config.ts ; indeed if I remove them locally then I observe the same issue ; So apparently there is something acting differently when using local dependencies versus published ones (even with 4.0.0-rc.1, I tried it)

Concerning your comment about having two instance of react-router:

• I don't understand why this would be different with the local packages versus the remote ones, since the peer dependency is missing with both versions
• Actually I did not even observe this myself (but that doesn't mean that it's wrong!) ; so I'm curious, how did you come to this observation?

Lastly, as you maybe already know, the no-code project is still a WIP to us and as such is not officially supported, that's why we can't prioritize the work on this module too much in comparison to supporting the v3 and finishing the release of the v4.

Anyways, thank you ever so much for the time you spent on this, and looking forward to hear from you again 🙂

[smeng9]

Hi @slax57

Thanks for your review.

As you mentioned,

For react 18, I will revert the change and only address one issue at a time.

For flashing problem, you are correct it is the hmr section and we will keep in this PR.

For vite.config.ts resolve aliases issue, I have the same result as you reproduced locally:

• using local dependencies does not have any issue
• using published 4.0.0-rc.1 dependencies by commenting the resolve section out have the issue. (same console output as https://github.com/marmelab/react-admin/pull/7469#issuecomment-1086154172)

For two instance of react-router, I have tested it and short conclusion is we need peerDependency

I don't understand why this would be different with the local packages versus the remote ones, since the peer dependency is missing with both versions

Even though peer dependency is missing with both version, the vite linking behaves differently. published 4.0.0-rc.1 is statically linked and the js file in dist folder is the final version, so unless we specify in peerDependency there will be two instances. local dependencies are dynamically linked as we only want it to resolve ra-* packages, not the react-router, so only one instance is created.

I'm curious, how did you come to this observation?

How I come to this observation is, I found the previous console output https://github.com/marmelab/react-admin/pull/7469#issuecomment-1086154172 does not make too much sense as the useNavigate hook is already inside <Router>. Then I searched react-router issue list and someone suggest https://github.com/remix-run/react-router/issues/8701 two instance causes the problem.

We can use following steps to verify: Set up

1. Comment out the resolve section in vite.config.ts to force it use published 4.0.0-rc.1 and we should experience the white screen problem

Test group 2. Add the react-router peer dependency to ra-no-code package and freshly rebuild the dist folder of ra-no-code (delete and recreate) 3. Copy the dist folder into no-code/node_modules/ra-no-code and remove no-code/node_modules/.vite cache 4. Run yarn dev on no-code project and we should not have any issue now.

Control group 5. Remove the react-router peer dependency to ra-no-code package and freshly rebuild the dist folder of ra-no-code (delete and recreate) 6. Copy the dist folder into no-code/node_modules/ra-no-code and remove no-code/node_modules/.vite cache 7. Run yarn dev on no-code project and we should have the issue again.

Lastly, I completely agree with you v4 official release will have a much higher priority. If there are not enough resource and time we can temporarily put it on-hold.