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

ivan-aksamentov commented 5 years ago

Bug Report

Describe the Bug

After upgrading to the latest versions of @ohif packages our app crashes with:

index.umd.js:25 Uncaught Error: Minified React error #321; visit https://reactjs.org/docs/error-decoder.html?invariant=321 for the full message or use the non-minified dev environment for full errors and additional helpful warnings. 
    at z (index.umd.js:25)
    at useContext (index.umd.js:25)
    at Sr (index.umd.js:49)
    at withI18nextTranslation(Unknown) (index.umd.js:49)
    at renderWithHooks (react-dom.development.js:15108)
    at mountIndeterminateComponent (react-dom.development.js:17342)
    at beginWork$1 (react-dom.development.js:18486)
    at HTMLUnknownElement.callCallback (react-dom.development.js:347)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:397)
    at invokeGuardedCallback (react-dom.development.js:454)

react-dom.development.js:19814 The above error occurred in the <withI18nextTranslation(Unknown)> component:
    in withI18nextTranslation(Unknown)
    in Tr
    in Unknown
    in div
    in t (created by ConnectFunction)
    in ConnectFunction (at Toolbar.tsx:216)
    in div (at Toolbar.tsx:202)
    in Toolbar (created by ConnectFunction)
    in ConnectFunction (at ViewerPage.tsx:13)
    in div (at ViewerPage.tsx:12)
    in ViewerPage (created by Context.Consumer)
    in Route (at PageSwitcher.tsx:15)
    in Switch (at PageSwitcher.tsx:13)
    in PageSwitcher (at App.tsx:30)
    in Router (created by ConnectedRouter)
    in ConnectedRouter (created by Context.Consumer)
    in ConnectedRouterWithContext (created by ConnectFunction)
    in ConnectFunction (at App.tsx:28)
    in Provider (at App.tsx:27)
    in App
    in DragDropContext(App) (at src/index.tsx:25)

the full text of the decoded error:

Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons: 1. You might have mismatching versions of React and the renderer (such as React DOM) 2. You might be breaking the Rules of Hooks 3. You might have more than one copy of React in the same app See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

What steps can we follow to reproduce the bug?

After upgrading from

    "ohif-core": "0.5.2",
    "react-viewerbase": "0.4.1",
    "ohif-cornerstone-extension": "0.0.30",

to

    "@ohif/core": "0.50.3",
    "@ohif/ui": "0.50.1",
    "@ohif/extension-cornerstone": "0.50.1",

In both cases

    "react": "16.9.0",
    "react-dom": "16.9.0",

Here is what I know so far:

If I remember correctly, the old versions predate React hooks
react-i18next might break Rules of Hooks. See react-i18next#934
After eliminating @ohif/i18n and react-i18next functionality from @ohif/ui completely the same error now appears in @ohif/ui (stack trace is readable up to the LayoutManager. @ohif packages only provide minified versions, so I cannot tell where the error happens exactly)
We don't have hooks in our app
This is a dev build of our app. It bundles a dev version of React 16.9.0. I don't know where the minified (prod) version of React comes from.
Nothing like that happened with old versions (probably due to lack of this particular check without hooks being present)

That's all information I've got so far. How can I debug this?

sedenardi commented 5 years ago

Have also encountered this. It might be more helpful to export an entry point into the package's main source code and let the consuming application about module bundling, rather than exporting a fully bundled and minified version.

dannyrb commented 5 years ago

Some Context:

It's a little tricky, but more than happy to discuss and pursue enhancements around this.

Right now, each extension and platform project is published independently w/ a UMD target. We can add CommonJS and Source, but ESM has to wait for WebPack v5, or we need to maintain a separate Rollup config.

@ohif/viewer is a bit special in that our monorepo setup has us building everything from source to create the bundle (UMD), or PWA output

The bundle, published as @ohif/viewer has two top level named exports:

App, the React component of the viewer.
installViewer, a convenience method that takes App's props (our config object), a container to render to, and a callback

This Issue

I think this above issue could be our package builds, or react-cornerstone-viewport / react-vtkjs-viewport, accidentally bundling react. LayoutManager would be using the react-cornerstone-viewport that is a dependency of @ohif/extension-cornerstone

We should, at a minimum:

[ ] Provide unminified bundles
[ ] Provide sourcemaps for minified bundles

There is an in-progress PR to shift LayoutManager into the Viewer project, as it doesn't make a ton of sense as a reusable UI component.

That's my brain dump

sedenardi commented 5 years ago

Right now, each extension and platform project is published independently w/ a UMD target. We can add CommonJS and Source, but ESM has to wait for WebPack v5, or we need to maintain a separate Rollup config.

Yeah it's a bit of a pain now, having been through it with some of my own packages. Didn't know if there was a specific technical reason for exporting it that way, or logistical.

I think this above issue could be our package builds, or react-cornerstone-viewport / react-vtkjs-viewport, accidentally bundling react. LayoutManager would be using the react-cornerstone-viewport that is a dependency of @ohif/extension-cornerstone

The issue isn't unique to @ohif/viewer. cornerstoneWADOImageLoader doesn't provide an ESM entry point either, which makes debugging (and even learning about the inner-workings) a bit of a pain. I've been able to get around the issue a little by cloning the repos into my development environment and importing from the source, but obviously not an ideal solution to deploy.

dannyrb commented 5 years ago

We don't have great visibility into how our packages are being consumed; so we optimize our use cases, and then wait for individuals to pipe up that it would be great if we also supported X.

Because we primarily touch the extension/platform packages through the monorepo, we don't feel the pain points of minified UMD consumption as keenly as others. I'm more than happy to review PRs for any OHIF or Cornerstonejs org repository packages that add ESM, Commonjs, and unminified variants. I can even provide some rough guidance.

All of this is "on my list" to do, but it's not at the top.

It's not super opaque, but our priorities are driven by grants, consultants contributing client work, developers addressing community needs to increase the appeal of the platform, and fly-by-the-night crusaders popping in a quick fix/enhancement.

sedenardi commented 5 years ago

Completely understand. Thanks so much for taking the time to answer.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dannyrb commented 5 years ago

👟 💨 🤖

I'm going to keep this open for now. We're cleaning up our UMD and package usage near future to address issues like this. I'll post back after we've made changes to get feedback on if this has been resolved.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jcosgrovehoppr commented 4 years ago

I am encountering the same error when I attempt to render the OHIF viewer within a React component.

componentDidMount() {
    var Viewer = window.OHIFViewer.App;
    var app = React.createElement(Viewer, ohifViewerConfig, null);
    ReactDOM.render(app, document.getElementById('viewer'));
}

Interestingly if I use the installViewer exported function ,I am able to render the viewer correctly. However my application requires the DOM element that the viewer is attached to, to be created and removed multiple times. This causes another error implying installViewer can only be called once per page. I went down the route of using the App object for this reason but cannot proceed because of the Invalid hook call error.

Perhaps there is a way to call installViewer multiple times per page? Or otherwise do you see a fix for this issue in the near future?

jcosgrovehoppr commented 4 years ago

Perhaps there is a way to call installViewer multiple times per page? Or otherwise do you see a fix for this issue in the near future?

I was able to use installViewer multiple times by editing the following Cornerstone code in the minified file. https://github.com/cornerstonejs/cornerstoneWADOImageLoader/blob/2420f321946b9c08442a7f82ed85155e1e9fec96/src/imageLoader/webWorkerManager.js#L153

Instead of throwing an exception, I logged the warning to the console and returned out of the function.

Using the App object would have encountered the same problem.

sedghi commented 1 year ago

Thank you for your past contributions and for raising this issue related to legacy versions of our OHIF Viewer. Your time and effort have been invaluable in helping us improve the product.

We wanted to update you that our development focus has shifted to OHIF Viewer v3, which is now available for testing on viewer-dev.ohif.org. This site is deployed from our master branch and incorporates new features, optimizations, and bug fixes. If you're still using viewer.ohif.org, it's worth noting that this is deployed from our release branch and may not have the latest updates. You can read more about our branching strategies here.

Given the move to Version 3, we are closing older issues that pertain to Versions 1 and 2, as we are no longer providing support for those versions. If you find that the issue you've raised is still relevant in Version 3, we encourage you to reopen this issue or create a new one for proper triage.

Thank you for your understanding and continued support.

OHIF / Viewers