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

fix: duplicated instances of react #6

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Conversation

PedroMiguelSS
Copy link
Contributor

@PedroMiguelSS PedroMiguelSS commented Apr 22, 2020

If you were using this template to create a component that was using react hooks, you might have faced this error:

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

You can read more about "Invalid Hook Calls" but long story short: we were having duplicated React module.

Why?
This is a common problem while using npm-link. The bundler was detecting two Reacts: one in our demo folder and the other one in our component folder.
Related issues:

Solution
To solve this problem 2 possibilities were considered:

  1. Create an alias on the demo's webpack config to use our component react and react-dom modules and remove them as dependencies from the demo's package.json file.
  2. Create a npm-link on our component to use the demo's react module.

This PR implements the solution 1. as it seems to be the best one. Solution 2. would require developers to run npm i inside demo/ before creating the npm-link. If we had chosen solution 2., the process could be either documented or semi-automated with npm scripts but it would be error-prone and we would be adding scripts to our final package (which IMO is not cool).

With the current solution, the work process is the same as it was before. It doesn't add any extra step or extra effort.

Copy link
Contributor

@AfonsoVReis AfonsoVReis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 i would only change the commit message to something else rather than problem with hooks usage since this is a problem related with how npm link handles react dependencies and not the hook usage itself.

@fsdiogo
Copy link
Contributor

fsdiogo commented Apr 22, 2020

I agree with @AfonsoVReis, I'd reword to something like fix: duplicate react dependency or something!

Copy link
Contributor

@fsdiogo fsdiogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link

@dominguesgm dominguesgm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@PedroMiguelSS PedroMiguelSS changed the title fix: problem with hooks usage fix: duplicated instances of react Apr 23, 2020
@paulobmarcos paulobmarcos merged commit 434ef54 into master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants