contiamo / operational-ui

Building blocks for effective operational interfaces
https://operational-ui.netlify.com
MIT License
258 stars 47 forks source link

Operational seems to have an issue with duplicating react #1362

Closed justinmchase closed 3 years ago

justinmchase commented 3 years ago

According to this documentation: https://reactjs.org/warnings/invalid-hook-call-warning.html#duplicate-react

I have duplicate reacts, which is a problem.

$ npm ls react
ipfsplay@1.0.0 D:\code\ipfsplay
+-- @operational/components@18.0.0
| `-- react@16.14.0
`-- react@16.14.0

Here are my dependencies:

  "dependencies": {
    "@operational/components": "^18.0.0",
    "emotion": "^9.2.12",
    "react": "^16.14.0",
    "react-dom": "^16.14.0",
    "react-emotion": "^9.2.12"
  }

And its giving me this error:

Invalid hook call. Hooks can only be called inside of the body of a function component.

It seems weird that operational has its own react if react needs to be top level. Whats going on and how can I fix it?

fabien0102 commented 3 years ago

Hi, regarding the error, you seams to have some hooks inside a class component or something like this. React seams to be the same version so I will be surprise if this is your issue (but I can be wrong of course).

Can you setup a quick codesandbox or repo with the problematic setup?

justinmchase commented 3 years ago

I'm trying to run it inside of Electron and I think its related to that. I'm still trying to debug it but if I find anything out I'll update here. The code is very simple, my hooks are working but something inside of operational components fails. Even a single page with a single component. I think I'll close this until I can find a better repro or an explanation and update with a solution in case anyone else ever sees this error.

justinmchase commented 3 years ago

I think the problem is that because its in Electron, the modules are getting loaded isolated like node modules would rather than all in the same global context like it would in the browser.

The error is here: image

And the callstack here: image

justinmchase commented 3 years ago

LOL I found this in react:

image

I'm thinking about using it to set the dispatcher 🤔

fabien0102 commented 3 years ago

I did use react with electron a long time ago (before hooks), was working. I will be very curious when you have a repro, we are not doing anything fancy there.

justinmchase commented 3 years ago

Yeah its definitely loading a second React and that is the problem. image

I followed the basic getting started here: https://www.electronjs.org/docs/tutorial/quick-start#quick-start-guide

I also installed npm i @operational/components

  "devDependencies": {
    "@types/node": "14.14.10",
    "@types/react": "^16.9.44",
    "@types/react-dom": "^16.9.9",
    "@types/react-router-dom": "^5.1.6",
    "electron": "^11.0.1",
    "typescript": "^4.0.5"
  },
  "dependencies": {
    "@operational/components": "^18.0.0",
    "react": "^17.0.1",
    "react-dom": "^17.0.1",
    "react-router-dom": "^5.2.0",
    "source-map-support": "^0.5.19",
  },

Then these simple files:

index.html

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8" />
  <title>Hello Electron React!</title>
</head>
<body>
  <div id="app"></div>
  <script defer lang="javascript">
    const React = require('react')
    const ReactDOM = require('react-dom')
    window.React = React
    window.ReactDOM = ReactDOM // purely for debugging

    require('@operational/components')
    require("./bin/index")
  </script>
</body>
</html>

src/index.tsx

import React from 'react';
import { render } from 'react-dom';
import App from './App';
render(<App />, document.getElementById('app'));

src/App.tsx

import React from 'react';
import { BrowserRouter as Router, Switch, Route, useHistory } from 'react-router-dom';
import { Button, HomeIcon, OperationalUI, Page, PageArea, PageContent, Sidenav, SidenavHeader, SidenavItem, useWindowSize } from "@operational/components"

const Hello = () => {
  return (
    <div className="Hello">
      <Button>🚀</Button>
    </div>
  );
};

export default function App() {
  const history = useHistory(); // error throws here as it tries to get the "current" dispatcher for the wrong instance of React
  const { height } = useWindowSize()
  return (
    <OperationalUI>
      <div style={{ display: "flex", height }}>
        <Sidenav compact>
          <SidenavHeader condensed label="Navigation">
            <SidenavItem
              label="Home"
              icon={HomeIcon}
              onClick={() => history.replace('/')}
            />
          </SidenavHeader>
        </Sidenav>
        <Page title="RSS">
          <PageContent areas="main">
            <PageArea name="main">
              <Router>
                <Switch>
                  <Route path="/" component={Hello} />
                </Switch>
              </Router>
            </PageArea>
          </PageContent>
        </Page>
      </div>
    </OperationalUI>
  );
}

tsconfig.json

{
  "compilerOptions": {
    "target": "ES2018",
    "module": "CommonJS",
    "lib": [
      "dom",
      "esnext"
    ],
    "declaration": true,
    "declarationMap": true,
    "jsx": "react",
    "strict": true,
    "pretty": true,
    "sourceMap": true,
    /* Additional Checks */
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    /* Module Resolution Options */
    "moduleResolution": "node",
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "resolveJsonModule": true,
    "allowJs": true,
    "skipLibCheck": true,
    "rootDir": "./src",
    "outDir": "./bin",
    "sourceRoot": "./src"
  },
  "exclude": [
    "bin",
    "node_modules"
  ]
}

Run i

npx tsc
npx electron .
$ node -v && npm -v
v14.15.4
6.14.10
justinmchase commented 3 years ago

Ok I'm dumb, that was the problem but the version numbers needed to be exact.

I did this:

npm i react@16.14.0 react-dom@16.14.0 --save-exact

And deleted node_modules and package-lock.json and then it worked. I think it was me not installing exact and not updating my package-lock.json file that put me in a weird state.

fabien0102 commented 3 years ago

Nice debugging session, I'm glad you found the issue! And yes, indeed, react & react-dom need to be sync (I also did learned this the hard way 😅)

justinmchase commented 3 years ago

I just encountered this again and the peer dependency versions in operational-ui are different than the actual versions it requires and it ends up causing this issue.

I'll file a different bug but for the record I had to dig into the package.json under my node_modules folder and find the actual versions in there and install those exactly.