preactjs / next-plugin-preact

Next.js plugin for preact X
394 stars 9 forks source link

Nextjs + Preact + Webpack 5 | Error: Hook can only be invoked from render methods #25

Closed tbgse closed 3 years ago

tbgse commented 3 years ago

Hi Preact team. First of all thanks for the amazing work 🌮

Describe the bug Whenever using any hooks with webpack 5 / next / preact I'm observing the following error when running the dev server:

Error: Hook can only be invoked from render methods.
    at Object.preact__WEBPACK_IMPORTED_MODULE_0__.options.__h (/home/tbgse/dev/webpack-5-test/.next/server/pages/_document.js:1281:7106)

and this error when trying to build

Error occurred prerendering page "/". Read more: https://err.sh/next.js/prerender-error
TypeError: Cannot read property '__H' of undefined
    at hooks_m (/home/tbgse/dev/webpack-5-test/.next/server/pages/_app.js:46:208)
    at hooks_y (/home/tbgse/dev/webpack-5-test/.next/server/pages/_app.js:46:580)

I'm not sure if this is a problem with preact itself or possibly with the next-plugin-preact. This bug does only occur after making the switch to webpack5. Before everything worked just fine. This is not an issue of misusing the hook outside of a render method, the same code is working just fine when using react or preact + webpack4.

To Reproduce I have created an example repo here: https://github.com/tbgse/preact-webpack5-next

This includes the following steps

Expected behavior Hooks should work correctly

marvinhagemeister commented 3 years ago

Some preliminary findings: The error is caused by module aliasing not working for some reason in node. The rendered DOM tree is passed to react-dom/server instead of preact/compat/server. Don't know why yet.

tbgse commented 3 years ago

really appreciate that you're looking into it so quickly. Do let me know if you need any further info or if there is anything I can do to help.

marvinhagemeister commented 3 years ago

Some more information: It's a combination of two issues:

tbgse commented 3 years ago

Hi @marvinhagemeister I saw that you merged some work to fix the duplicated copy of Preact. When testing, did that fully resolve the issue or do we indeed also need to wait for more adjustments to work with the new eager loading of react / react-dom for this issue to be resolved?

marvinhagemeister commented 3 years ago

@tbgse The eager aliasing is not working yet.

tbgse commented 3 years ago

@marvinhagemeister thanks for the update. Do you already know how much effort this will be to resolve? I'm trying to come up with a timeline for us to migrate to webpack 5. The problem is that we cannot upgrade to the latest versions of next because prefresh is not supported in webpack 4 mode with the latest versions. So we're a few minor versions behind with next now already.

Again if you already have a hunch how this problem could be solved, let me know and I'm happy to dig a bit myself too.

JoviDeCroock commented 3 years ago

Hey @tbgse

Prefresh supports next v10 with webpack 4 we have a full test suite for both v5 and v4

tbgse commented 3 years ago

hi @JoviDeCroock then i might have misunderstood our conversation here: https://github.com/JoviDeCroock/prefresh/issues/274

webpack 4 + next 10.0.6 upwards still fail for us with the errors described in the issue. I assumed that the right way to go here would be moving to webpack 5 (which in return lead to the issues described here).

Let me know if I missed out on a way to make it work with webpack 4.

JoviDeCroock commented 3 years ago

Adding webpack as a dep does the trick normally, just adding webpack 4. Oddly enough as of latest next it works for me without

You can check the prefresh/test/fixture/next folder for webpack 4 and next-webpack5 for webpack 5

tbgse commented 3 years ago

@JoviDeCroock ugh, i was so focused on solving this with webpack 5 that i never thought about actually adding v4 as local dependency 🤕 it works perfectly even with latest next after adding it, thanks!

I'd still appreciate it, if this issue could be kept open just because it'd be quite nice to be able to move to webpack 5 eventually.

JoviDeCroock commented 3 years ago

@tbgse well webpack 5 works as well, it just has one issue with styled-components which I haven't gotten to the bottom of as it works with normal webpack 5 but fails when next 10 is involved.

But generally you should be able to just migrate to Next10 Webpack 5 as well, my tests run well in the prefresh repo. https://github.com/JoviDeCroock/prefresh/tree/main/test/fixture/next-webpack5

tbgse commented 3 years ago

@JoviDeCroock oh i was referring to this issue with the preact plugin that marvin mentioned above where:

Next now loads both react and react-dom eagerly, before we are able to set up any aliasing

which is an issue with this plugin rather than prefresh.

A bit off topic, but do you have more info on the styled-components issue with prefresh that you're looking into? We've also noticed a problem with styled components locally where they don't update with HMR but require a hard refresh. Could this be related?

JoviDeCroock commented 3 years ago

@tbgse the issue is now on their radar as well :)

https://github.com/vercel/next.js/issues/23130 https://github.com/vercel/next.js/discussions/23512

tbgse commented 3 years ago

awesome, i'll keep my eyes on those two open issues 👀

pierremouchan commented 3 years ago

Same issue ! hope it can be resolved soon !

joaogarin commented 3 years ago

Does the plugin support webpack 5 at this point?

tbgse commented 3 years ago

Just saw that https://github.com/vercel/next.js/issues/23130 was merged on the latest canary release, curious to see if that work unblocks this issue to be investigated further @JoviDeCroock ?

JoviDeCroock commented 3 years ago

It should resolve the issue, haven't been able to test but using latest canary should work

tbgse commented 3 years ago

Yes i can confirm it works nicely for our codebase now with no additional config required 🥳 happy to close this issue

joaogarin commented 3 years ago

I am getting this error https://github.com/JoviDeCroock/prefresh/issues/274#issuecomment-800290828 "Error: Cannot find module 'webpack/lib/dependencies/ConstDependency'" When I try webpack 5, is this a nextJs issue that then should be fixed in a minor release you think? Would be great to start maybe finding other issues (I too use styled components so maybe that will also give its own issues) even with latest fixed in that PR I seem to still have the same issue

tbgse commented 3 years ago

@joaogarin have you added webpack5 as local dependency to your own project? This did the trick for me.

joaogarin commented 3 years ago

Thanks that does seem to do something, now I get a different error, still from @prefresh which is

Error: Can't detect valid entry point.
    at injectEntry (/---/node_modules/@prefresh/webpack/src/utils/injectEntry.js:27:9)
tbgse commented 3 years ago

We do have the same error now, whenever we run on latest next@canary, i was about to look into it and post a new issue for this error. What next version are you on? for 10.2 it should work fine?

joaogarin commented 3 years ago

ah! indeed ;) With 10.2 it does work :exploding_head: now I am wondering..will 10.3 break it :laughing: but great to know that its at least not far :+1: Thanks for the help!

JoviDeCroock commented 3 years ago

I'll check later today whether they have started wrapping entry points in some form that isn't known by Prefresh yet 😅 currently prefresh supports all webpack-compatible entries

tbgse commented 3 years ago

@JoviDeCroock is fixing these things faster than I can open issues 😄

We are also observing an odd bug right now after moving to webpack 5 where some useContext hooks are throwing errors after saving a change to any file that'll trigger HMR runs, e.g. https://gyazo.com/541e136e8bc00a3ef1a7daeb3083c86f almost feels like page props are becoming undefined or so, which hasn't happened on wp4.

Curious to see if this is happening to someone else. Looking into this a bit more today and trying to set up a repo to reproduce.

JoviDeCroock commented 3 years ago

What surprises me is that react-refresh as a file is being included again I've done everything I possibly can to disable react-refresh as a whole in @prefresh/next but it keeps finding new ways to be injected.....

tbgse commented 3 years ago

This is on 10.2 stable, let me know if there is anything i can do to help debug this. I'll try to prepare a repo if needed.

Edit: this might be somehow related to how we are using a HOC to have a shared wrapper for all pages e.g.:

function TestPage(props: any) {
  return (
    <>
      <TestComponent />
    </>
  );
}

export default withApp(TestPage);

it seems like for some reason withApp does not receive any props on HMR.

wilsonpage commented 3 years ago

I'm seeing some strange behaviour on navigation. If a page hasn't been compiled before, it navigates and then jumps back to the previous page. Navigating subsequent times behaves as expected.

I'm trying this config to get benefits of Webpack 5 in production but w/o prefresh/hmr bugs in dev:

future: {
  webpack5: NODE_ENV !== 'development',
},
TheMrZZ commented 3 years ago

I'm still getting the Error: Hook can only be invoked from render methods with NextJS 11 and Preact. This issue probably should be reopened.

Stack trace:

Error: Hook can only be invoked from render methods.
    at Object.n.options.__h (C:\Users\me\project\node_modules\preact\debug\dist\debug.js:1:6255)
    at p (C:\Users\me\project\node_modules\preact\hooks\dist\hooks.js:1:168)
    at Object.x (C:\Users\me\project\node_modules\preact\hooks\dist\hooks.js:1:323)
    at ReactDevOverlay (C:\Users\me\project\node_modules\@next\react-dev-overlay\lib\internal\ReactDevOverlay.js:87:27)
    at ReactDevOverlay (C:\Users\me\project\node_modules\next\dist\server\next-dev-server.js:2:181)
    at processChild (C:\Users\me\project\node_modules\react-dom\cjs\react-dom-server.node.development.js:3353:14)
    at resolve (C:\Users\me\project\node_modules\react-dom\cjs\react-dom-server.node.development.js:3270:5)
    at ReactDOMServerRenderer.render (C:\Users\me\project\node_modules\react-dom\cjs\react-dom-server.node.development.js:3753:22)
    at ReactDOMServerRenderer.read (C:\Users\me\project\node_modules\react-dom\cjs\react-dom-server.node.development.js:3690:29)
    at renderToString (C:\Users\me\project\node_modules\react-dom\cjs\react-dom-server.node.development.js:4298:27)

Dependencies:

{
    "next": "^11.0",
    "next-compose-plugins": "^2.2.1",
    "next-plugin-preact": "^3.0.6",
    "preact": "^10.5.13",
    "preact-render-to-string": "^5.1.19",
    "react": "npm:@preact/compat",
    "react-dom": "npm:@preact/compat",
    "react-ssr-prepass": "npm:preact-ssr-prepass",
    "webpack": "5.39.0"
}
tbgse commented 3 years ago

We have already upgraded to Next 11 in our codebase and are not seeing that issue with webpack 5 + preact, not sure if this is related. I have updated my small test repo here to Next 11: https://github.com/tbgse/preact-webpack5-next and it seems to work fine with hooks.

adamduncan commented 2 years ago

@tbgse Are you using yarn or npm? I notice this repo has both lock files.

Interestingly, trying this reduced test case (a copy straight from canary's using-preact example), I'm still experiencing some unpredictable results using npm, both locally and when deploying to Vercel.

Trying to home in on exactly what combination of npm and Next versions allow me to reproduce the Error: Hook can only be invoked from render methods when running dev (and even a Error: Cannot find module 'react' when running build).

tbgse commented 2 years ago

@adamduncan the yarn file was just something leftover from the initial next init of the repo, I just removed it for clarity. Something i noticed looking at your repo is that you did not specify the version for the compat references in package.json. I remember having to do this to make things work:

    "react": "npm:@preact/compat@0.0.4",
    "react-dom": "npm:@preact/compat@0.0.4",

Hope this helps. Your next-plugin-preact version also seems to be behind. The latest version is at 3.0.6 so installing it at that version might make a difference too. Same for preact itself and preact-render-to-string. Try installing those at the latest version too.

adamduncan commented 2 years ago

Thenks @tbgse. You're right, using the versions as you have in your repo build and run as hoped when using npm v6.

Interestingly though, with that combination of packages I'm seeing a peer dep issue when using npm v7?

Screenshot 2021-07-23 at 21 13 38

Whereas if I build and run the app with the using-preact example's deps on npm v7 build is fine, but I can replicate Error: Hook can only be invoked from render methods at runtime.

Which version of npm are you using?

tbgse commented 2 years ago

I'm using v6. I can try to have a look tomorrow why v7 isn't working. We've been having some issues with npm v7 that are unrelated to preact in our real world repo so I never tried it with v7 before.

tbgse commented 2 years ago

@adamduncan i can confirm that i just tried to run this with npm7 and ran into exactly the same issue as you. Will update you if I find a solution

tbgse commented 2 years ago

@JoviDeCroock is this something any of you guys might be able to help look into (see the screenshot posted above)

JoviDeCroock commented 2 years ago

Yes we need to add something to our peerDeps, as an inbetween you can use npm install .... --legacy-peer-deps

tbgse commented 2 years ago

Thanks for the super fast response, at least that means we haven't been doing anything wrong 😁

tbgse commented 2 years ago

Hi @JoviDeCroock just wanted to check in if anything has changed already regarding the peer deps so we don't need to use legacy mode anymore?

JoviDeCroock commented 2 years ago

@tbgse you should be able to bump your @preact/compat package to 17

jherjati commented 2 years ago

I'm using the latest nextjs example using-preact, still get either this error Hook can only be invoked from render methods or Cannot read property 'context' of undefined depends on how I import useState, whether from "preact/compat" or "react", respectively. Maybe there is a race condition because it only shows up when my app grows (use many hooks).

apfelbox commented 2 years ago

Same issue here. Can't get it to work neither

mraxime commented 2 years ago

I get the same issue as soon as I use react-hook-form.

kucira commented 2 years ago

Hi there, same with me, got the same error when using react-hook-form.

Ethaan commented 1 year ago

@apfelbox @maxime50 and @kucira and for anyone facing this issue with react-hook-form, see this comment

(try adding experimental.esmExternals: false to next.config.js)