vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.92k stars 26.68k forks source link

Invalid hook call in 9.0.6 #9022

Closed baldurh closed 2 years ago

baldurh commented 4 years ago

Bug report

Describe the bug

When you use react a component residing outside the main Next.js project folder which uses hooks. You end up getting Invalid hook call error and the application breaks. Components without hooks work as expected.

The bug appears in all versions >9.0.5 when you change the webpack config so that files outside the main folder are transpiled. It’s working fine in <=9.0.5

To Reproduce

Check out the repro at https://github.com/baldurh/next-9.0.6-bug-repro

Expected behavior

The code should not break when using files outside the project folder.

System information

Additional context

I know this is probably not a common use of Next.js but in our project we’re using a monorepo and have a shared folder with components used by multiple applications. It would be nice to get this working again. If someone finds an alternative config we could use I’d also be happy to do that 🙂

lfades commented 4 years ago

@baldurh This is indeed very uncommon, when using platforms like Now only the folder where the Next.js app lives is deployed, it's better that way because otherwise you would need to know about all the external modules first. 2 better alternatives are:

baldurh commented 4 years ago

@lfades thanks for the reply. Neither of those options are available to us and we're not deploying to Now or anything similar. We used yarn workspaces initially but then we integrated bazel and it does not play nicely with the symlinking nature of yarn workspaces. Npm packages means we cannot develop the shared modules as fast as we like. It's just too much overhead.

isaachinman commented 4 years ago

@baldurh Just ran into this with next-i18next as we've got nested NextJs apps as examples. Did you find a workaround?

baldurh commented 4 years ago

@isaachinman We have not. We haven’t managed to upgrade to 9.x yet because of other reasons so I haven’t been looking into it. Does anyone have an idea of where the code affecting this might be? I’d love to understand the problem better.

isaachinman commented 4 years ago

I haven't had time to dig into this yet, but if anyone needs a repro: clone next-i18next, cd into examples/simple and upgrade the NextJs version to >=9.0.6.

It's currently on 9.0.3, so this is technically a breaking change on a patch.

jaredcwhite commented 4 years ago

I had a similar error come up recently and had to downgrade to 9.0.5 (and React 16.8.x). I sort of narrowed down the problem I saw to Next's use of MDX, but I don't have any concrete details beyond that.

felixmosh commented 4 years ago

I've dug into the same issue with a pretty big project based on Next & next-i18next.

I saw that this error can be thrown by 3 reasons:

  1. Misaligned react & react-dom versions - not applied to my app
  2. 2 versions of react-dom imported - not applied to my app
  3. Improper use of React hooks - I don't use hooks but some libs are, and it looks like it works for every one else.

The strange thing is that it happens only on production build.

baldurh commented 4 years ago

@timneutkens @Timer sorry for tagging you in this but I’d love some input from you. Do you think this is something that could be fixed do we all need to implement some workarounds? This is a pretty big blocker for us at the moment. Thanks.

Timer commented 4 years ago

It seems you aliased react but not react-dom. Can you try that?

baldurh commented 4 years ago

@Timer Thanks. I tried but it did not have any effect

baldurh commented 4 years ago

I was able to resolve this just now in the repro by moving the react and react-dom dependencies one level up. I just pushed the changes if anyone wants to try it out. I haven’t tried it on our actual project but I’m hopeful it will work for us. Perhaps this could solve the issue for @isaachinman, @jaredcwhite and @felixmosh as well?

baldurh commented 4 years ago

@Timer I got this working in our project but I also had to make sure I had no other dependencies which installed react into our projects’ node_modules folder. In our case it was related to storybook (yarn why react helped a lot 😄). We were planning to move storybook to a separate project anyway so I think this solution will work in our case.

Timer commented 4 years ago

Ah, yes. Improperly published node_module packages will cause this (with dependencies on react(-dom|) instead of peer dependency).

felixmosh commented 4 years ago

I was able to resolve this just now in the repro by moving the react and react-dom dependencies one level up. I just pushed the changes if anyone wants to try it out. I haven’t tried it on our actual project but I’m hopeful it will work for us. Perhaps this could solve the issue for @isaachinman, @jaredcwhite and @felixmosh as well?

Can you elaborate regarding the changes that you have done in this repo?

I run npm ls react or npm ls react-dom I got only my next app in the list.

baldurh commented 4 years ago

@felixmosh Sorry, apparently the push failed for me yesterday. Now the changes are definitely there 😅 I moved react and react-dom from the app folder into the root folder so now you need to do yarn/npm install in both the app folder and the root folder before you run the app. Hope this is clear enough.

We’re going to have to do some changes to our build system to get this working in production so this solution is still a bit of a hassle for us 😝

felixmosh commented 4 years ago

Thanx for the explanation, I will wait for Next team to solve it, sounds a bit weird to put react deps on the root of my mono-repo...

baldurh commented 4 years ago

@felixmosh Yeah I kind of agree with you. However, if you use something like yarn workspaces that’s exactly what that tool will do. If you have the same dependency in two or more projects it will hoist the dependencies to the root. It’s nice because it ensures that you have the same version of the dependencies in all your projects. But if you don’t have a tool like that you’d have to manage it yourself which is indeed a bit awkward. I agree the best solution would be that the Next.js team takes a look and solves this for all of us 😇🙏🏻

bryan commented 4 years ago

Running into the same issue, and bringing react and react-dom one level up and running the server from the root is the only workaround that works currently on 9.1.5. Linking https://github.com/facebook/react/issues/13991 and https://github.com/facebook/react/issues/15315#issuecomment-479802153 for reference as I found those links before this issue.

creativej commented 4 years ago

Hi any update on this issue? We have a monorepo and we are encountering this exact issue.

nodkz commented 4 years ago

Meet with the same problem. v9.0.5 works great with hooks for components imported outside the rootFolder.

Starting from 9.0.6 till 9.1.6-canary.5 have the same problems.

The problem occurs only on the server-side. If SSR disabled (eg. load external component via dynamic) then all works as expected for versions >=9.0.6.

felixmosh commented 4 years ago

@nodkz it is an issue with react package resolving, therefore it happens only in node.

felixmosh commented 4 years ago

@Timer this issue got move to the "next" milestone for around 6 versions, it prevents me from updating to the latest version.

I've wasted entire day of time in accommodation on it, don't know what is the source issue of it, even tried the workaround (which didn't worked).

Do you need any help of investigating any direction? Do you have any gut-feeling about it? Why is that happens only on production build? What was changed from 9.0.5 to 9.0.6 that may be related to this?

Thanx 🙏🏼

felixmosh commented 4 years ago

I think that I've found the issue!!! I think that it is a combination of 2 things:

  1. usage of sym-link for node_modules
  2. i18next / react-i18next were not external in server bundles!! image In my case, when it explodes on production build, it complains on i18next useTranslation hook...

So i've investigated the reason for why there are node modules inside server bundles at all (best practice for server bundles is to make them externals).

I saw that next has some exceptions for next lib (why?), the funny part is that this regex catches all libs that ends with next/dist/, as i18next & react-i18next does!!

So, if you change this:

res.match(/next[/\\]dist[/\\]/) 

into

res.match(/[/\\]next[/\\]dist[/\\]/) 

The server bundle will exclude all libs that are not next & ends with next/dist, and it solved the issue!

baldurh commented 4 years ago

For me the main issue is the new way requests are resolved: https://github.com/zeit/next.js/blob/canary/packages/next/build/webpack-config.ts#L446

Since we have components outside the project root the request resolution will throw an error which results in react being bundled in the server chunks. And that’s why we get the Invalid hook call error on the server.

Timer commented 4 years ago

@baldurh context in that expression you linked is provided by webpack, and is different that the compilation root (your project directory). It's always the directory of the file that's issuing the require.

baldurh commented 4 years ago

Right. I’ve patched this to make it work for us for now. I think we’ll eventually change the structure of the code so that the dependencies are shared on an upper directory level. However, even if react is available in the external folder (outside the root) I still got the error.

karolis-sh commented 4 years ago

I'm trying to use a linked package and I'm experiencing the same issue. Sadly none of the fixes from https://github.com/facebook/react/issues/13991 work 🙁

hyphmongo commented 4 years ago

I'm also experiencing the same issue with a component library symlinked with yarn link. This worked fine up until v9.0.6-canary.4 and now I'm in the same position as some other commenters and cannot upgrade past this.. I have pin-pointed the change to this PR https://github.com/zeit/next.js/pull/8739

My component library uses react, react-dom, and styled-components. The configuration for this is as follows

Update

I was able to fix this by including these modules in the server externals. Thanks to @HosseinAgha in this comment https://github.com/martpie/next-transpile-modules/issues/50#issuecomment-558318688

if (isServer) {
  config.externals = [
    'react',
    'react-dom',
    'styled-components',
    ...config.externals
  ]
}
raphaelHuerzeler commented 4 years ago

I'm seeing the exact same issues, none of the workaround worked for me so far.

My package works if I publish it and install it (and use resolve.alias in my next.config.js).

But running a dev build with the package linked via yarn link @mypackage always results in an invalid hook error.

I was also able to get it working by modifying node_modules/dist/build/wepack-config.js and commenting out the lines added in https://github.com/zeit/next.js/pull/8739

What I'm seeing if I log baseRes and res is that the if condition triggers as:

Update:

We managed to work around the problem by converting our package to use yarn workspaces (even though our repo only contains a single package). We moved our code from ./src to ./package/our-package-name/src and setup yarn workspaces => https://classic.yarnpkg.com/en/docs/workspaces/

This works around the problem as this will hoist common dependencies to the top level ./node_modules folder and ./package/our-package-name/node_modules will remain largely empty.

So now when we link our package next won't pull in a second version of react anymore (as it's not present in our packages node_modules folder) and everything works as it should.

jhvissotto commented 4 years ago

I have the same fucking problem. ¬¬'

timneutkens commented 4 years ago

We generally collapse swearing as it's against the code of conduct.

jhvissotto commented 4 years ago

Sorry, I was angry with this bug. Actually, I like too much of the Next.JS. But no can use it cause this issue is boring.

martpie commented 4 years ago

We are facing this issue when working with external local packages and next-transpile-modules.

As we want to stick with the latest version of Next.js, I will try to submit a patch to Next.js if I find the root cause.

qlereboursBS commented 4 years ago

I face the same issue, after installing next-i18next@4.2.1 My dependencies are next@v9.3.1, react@16.13.0, react-dom@16.13.0 and of course many other libs (but everything was working before installing next-i18next). If someone has a workaround, it could be great :+1:

johot commented 4 years ago

Thank you for posting this issue, I also had problems with symlinking our design system (react component library) package. We are also transpiling it using https://github.com/martpie/next-transpile-modules.

The fix as suggested here worked for us:

// next.config.js
const nextConfig = {
    webpack: (config, options) => {
        // modify the `config` here

        if (options.isServer) {
            config.externals = ["react", ...config.externals];
        }
        config.resolve.alias["react"] = path.resolve(__dirname, ".\\node_modules\\react");

        return config;
    },
};
// more plugins etc...

Our alternative workaround that requires no config

But would be nice to see this fixed in NextJS, I spent so much time trying to understand why webpack alias worked for all my non NextJS project :)

PS. I have no idea how this would affect a production build but we would only use this during dev

timneutkens commented 4 years ago
if (options.isServer) {
            config.externals = ["react", ...config.externals];
        }

react is already external server-side.

config.resolve.alias["react"] = path.resolve(__dirname, ".\\node_modules\\react");

This won't solve the issue.

As said before this issue is related to your dependencies depending on react whereas they should have a peerDependency on react (and react-dom if they need it).

johot commented 4 years ago

@timneutkens

Well no this is not always the case. I for sure has react and react-dom as peer dependencies. The problem still occurs though if you for example symlink your library to a nextjs project. What then happens is that you will have a node_modules folder inside your library (at least if you ever ran npm i or npm link in that library folder).

When react gets resolved from this library folder it will resolve to the one in that node_modules folder and you get dual copies of react causing the problem. If I delete the node_modules folder inside my lib or install it using something else than npm link then yes ofc it works (if you use peer dependencies or exactly the same react version).

So to solve this during dev you want to be able to alias react to force everyone to use the same version. Because of the problems mentioned here this has no effect in the current NextJS version without adding the config.externals ... part (at least for me), probably as people have mentioned here because of some change as noted here #8739 ?

rosgoo commented 4 years ago

A similar issue is happening to me but (potentially) because of the material-ui library (as outlined in #10162), my temporary fix for now was just to add npm run cleanin my preserve and dev scripts as outlined here: https://github.com/zeit/next.js/issues/10162#issuecomment-612501431

@timneutkens I understand the real issue has to with how those dependencies are listing their own deps (deps vs peer-dep) but any idea what we can do in our own app to fix this more permanently?

johot commented 4 years ago

@ryan-0 Have you any special setup? Would be surprised if material Ui doesn’t list react as a peer dep? Like do you use a very old react version or symlink anything?

rosgoo commented 4 years ago

no special setup.. no symlinking and react 16.13.1 --> we have some other deps that might be causing the issue to be fair, but at least according to that repro it seems maybe related to material-ui/core (which we also use): https://github.com/zeit/next.js/issues/10162

johot commented 4 years ago

@ryan-0 is there a node_modules folder with react inside your material-ui folder?

Also does is start working after running npm dedupe?

rosgoo commented 4 years ago

no it doesn't look like there is a nested node folder, which is why I'm confused as to how the bug is happening. and no npm dedupe did not work :(

aleclarson commented 4 years ago

Strangely, using resolve.alias does not seem to affect packages outside the project root.

Here's my next.config.js file:

const path = require('path')

module.exports = {
  webpack: config => {
    const { alias } = config.resolve || {}
    alias.react$ = path.resolve('node_modules/react')
    alias['react-dom$'] = path.resolve('node_modules/react-dom')

    config.resolve = {
      ...config.resolve,
      alias,
    }

    return config
  }
}

I'm using yarn link with a local package that exists in a Lerna monorepo. Its node_modules does not contain a copy of react, but the monorepo root does. I wouldn't expect that to make a difference as long as resolve.alias is used, but that's unfortunately not the case. After removing the copy of react from the monorepo root, I'm now getting a Cannot find module 'react' error instead.

fernandoabolafio commented 4 years ago

Did someone find a good solution for this?

I have a linked next library which I am using next-transpile-modules to have it added to my 'consumer' project. I added the react alias in my next.config.json as mentioned in their docs but it wasn't enough. I am still getting the error of duplicated dependencies for React.

aleclarson commented 4 years ago

You could try using relative-deps by @mweststrate

johot commented 4 years ago

Did someone find a good solution for this?

I have a linked next library which I am using next-transpile-modules to have it added to my 'consumer' project. I added the react alias in my next.config.json as mentioned in their docs but it wasn't enough. I am still getting the error of duplicated dependencies for React.

Yes see my post above you need to add the config.externals part in my sample, then alias starts working again

fernandoabolafio commented 4 years ago

@johot I tried your solution but it didn't quite work for me. I started getting some weird errors, but mainly this one: cannot destructure property 'query' of 'Object(...)(...)' as it is null after trying your solution. The object seen as null in this case is the useRouter from next/router.

@aleclarson Thanks for the tip. I will give it a try if I can't make it work with the next setup. Are you using it currently?

martpie commented 4 years ago

If you are using next-transpile-modules and Yarn, the solution is quite straightforward: https://github.com/martpie/next-transpile-modules#i-have-trouble-with-duplicated-dependencies-or-the-invalid-hook-call-error-in-react

If you are using npm, I am still myself looking for a solution :c

fernandoabolafio commented 4 years ago

Ok, so my final solution was to migrate from yarn link to yalc. I have a gulp task which watch for files changes and copy the files to my dist folder and then pushes the changes to the yalc store.

In my 'consumer' I modified the tsconfig.json to resolve the paths like this:

 "paths": {
      "~/*": ["/src/*"],
      "my-library/*": ["./node_modules/my-library/dist/*"]
    },

and in the next.config.js I added the following:

 experimental: {
      jsconfigPaths: true, // enables it for both jsconfig.json and tsconfig.json
    }

So next can resolve the paths based on the tsconfig.json paths. More info here.

Long story short: combining yalc + next-transpile-modules improved a lot my local development setup. No duplicate dependencies and weird errors. The behavior of directly adding the module using yarn add and linking the module with yalc are pretty much the same.

josebrito commented 4 years ago

If you're using a locally linked library that depends on styled-components, refer to this: https://styled-components.com/docs/faqs#linking-in-an-ssr-scenario

In server/index.js:

const moduleAlias = require('module-alias');
moduleAlias.addAlias('styled-components', path.join(__dirname, '../node_modules/styled-components'));

But, we also needed to add the following in next.config.js:

config.resolve.alias['react'] = path.resolve(__dirname, './node_modules', 'react');
config.resolve.alias['react-dom'] = path.resolve(__dirname, './node_modules', 'react-dom');
config.resolve.alias['prop-types'] = path.resolve(__dirname, './node_modules', 'prop-types');
config.resolve.alias['styled-components'] = path.resolve(__dirname, './node_modules', 'styled-components');

Hope it helps.


Tested with:

Next: 9.3.5 React: 16.13.1 styled-components: 5.1.0