Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web Chat throwing "Invalid Hook" error with React Hooks #2261

Closed
tdurnford opened this issue Aug 1, 2019 · 1 comment · Fixed by #2274
Closed

Web Chat throwing "Invalid Hook" error with React Hooks #2261

tdurnford opened this issue Aug 1, 2019 · 1 comment · Fixed by #2274
Assignees
Labels
bug Indicates an unexpected problem or an unintended behavior.

Comments

@tdurnford
Copy link
Contributor

Screenshots

image

Version

v4.5.0 built from master branch

Description

The development team recently bumped Web Chat's React dependency to v16.8.6 which introduces React Hooks to Web Chat. While working on a pull request for the typing indicator, I tried to use the useState and useEffect hooks in a component; however, Web Chat threw an "Invalid Hook" Error. I created a really simple component to double check it wasn't my logic, but Web Chat still threw the error.

To Reproduce

Add a simple component with React Hooks to Web Chat.

const TypingIndicator = () => {
  const [item] = useState('Hello, World!');
  return <div>{ item }</div>
}

Expected behavior

React Hooks should not cause Web Chat to crash.

[Bug]

@tdurnford tdurnford added bug Indicates an unexpected problem or an unintended behavior. Pending labels Aug 1, 2019
@compulim
Copy link
Contributor

compulim commented Aug 6, 2019

After some investigations, the core reason is, Webpack doesn't play well with npm link (via lerna), thus, multiple copies of react are being loaded. And React hooks doesn't like multiple copies of React.

  • We should not include react and react-dom in bundle as dependencies in any case (we are component package, should never bring the framework)
    • We should include react and react-dom as peer dependencies and requires the application to include them
  • We should include react in the root package. The root package contains all devDependencies which is similar to "manually hoisted packages"
    • We try not to use hoist in lerna because it conflict with Babel bridge
    • If we hoist, we will have two packages of same name, babel@7.0.0-bridge.0 vs. babel@6, and lerna prefer 6, which broke Jest because it use the bridge
  • We cannot include react in playground as well
    • For unknown reason, Webpack dev server in playground tried to load two copies of React
      1. Web Chat bundle/component, it load the react from root /node_modules
      2. Playground app, it load the react under /packages/playground/node_modules
    • If we rename/remove the React package at root, Webpack dev server works fine, but we shouldn't

Some package dependencies cleanup is needed to enable React hooks in a monorepo with both component and app. It's a known issue across the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or an unintended behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants