preactjs / next-plugin-preact

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

Problems when installing with latest NextJS #22

Closed gergelypap closed 3 years ago

gergelypap commented 3 years ago

When I install the required packages, two problems happen.

First, the vulnerability fixes would introduce breaking changes.

$ npm install --save next next-plugin-preact preact react@npm:@preact/compat react-dom@npm:@preact/compat react-ssr-prepass@npm:preact-ssr-prepass preact-render-to-string

up to date, audited 1049 packages in 11s

50 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (3 low, 4 high)

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

After this, I update my next.config.js:

const withPreact = require("next-plugin-preact");

module.exports = withPreact({});

When building the project, this happens:

$ npm run build

> lofasz@0.1.0 build
> next build

----------------------------------------------------------------------------------------------------------------------------------------------
[preact] Missing/incorrect dependencies.
Please run:
  npm i react@npm:@preact/compat react-dom@npm:@preact/compat

or:

  yarn add react@npm:@preact/compat react-dom@npm:@preact/compat
----------------------------------------------------------------------------------------------------------------------------------------------

Needless to say, these dependencies are already present. I honestly don't know what's happening. Tested with latest, FRESH install of NextJS.

My package dependencies:

 "dependencies": {
    "next": "^10.0.6",
    "next-plugin-preact": "^3.0.3",
    "preact": "^10.5.12",
    "preact-render-to-string": "^5.1.12",
    "react": "^17.0.1",
    "react-dom": "^17.0.1",
    "react-ssr-prepass": "npm:preact-ssr-prepass@^1.1.3"
  }

UPDATE: Now I tried with these dependencies:

    "next": "10.0.0",
    "next-plugin-preact": "^3.0.3",
    "preact": "^10.5.5",
    "preact-render-to-string": "^5.1.11",
    "react": "npm:@preact/compat@0.0.3",
    "react-dom": "npm:@preact/compat@0.0.3",
    "react-ssr-prepass": "npm:preact-ssr-prepass@^1.1.2"

Deleted node_modules and package-lock.json, and npm i says:

$ npm i
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: test@0.1.0
npm ERR! Found: react@0.0.3
npm ERR! node_modules/react
npm ERR!   react@"npm:@preact/compat@0.0.3" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.6.0 || ^17" from next@10.0.0
npm ERR! node_modules/next
npm ERR!   next@"10.0.0" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!

Versions and stuff:

Ebernn commented 3 years ago

Have you found a temporary solution? @gergelypap 🤔

jnv commented 3 years ago

npm 7 changed the handling of peer dependecies to install them by default which will likely be issue here since many React-specific packages specify React as a peer dependency. My suggestions:

ndimatteo commented 3 years ago

Same issue here, I'm on Next 10.0.9 and tried a clean install with --legacy-peer-deps and the error persists.

I've also tried this approach, and it doesn't work either.

Anyone have a solution here? There seems to be huge blockers to getting preact to play nice with Next.

heyitsarpit commented 3 years ago

So I just did an upgrade but without chaging the config, ie. did not include withPreact() in next.config.js and it worked.

ndimatteo commented 3 years ago

@onygami if you aren't including your config export with withPreact() then... you're not using Preact...

Just installing it with NPM is not going to make your app use Preact, that's the whole point of the withPreact() wrapper.

heyitsarpit commented 3 years ago

@onygami if you aren't including your config export with withPreact() then... you're not using Preact...

Just installing it with NPM is not going to make your app use Preact, that's the whole point of the withPreact() wrapper.

Exactly that's what I thought, but then I built the project and my bundle size decreased by almost 40kb. I verified by rolling back the changes and bundle size went back being 40kb bigger. Then I installed preact/debug ran the profiler in the browser extension. It shouldn't work, but it does.

ndimatteo commented 3 years ago

@onygami Thinking about this more, did you alias react in your package.json?

If so, that would explain how it's using preact over react, despite not updating the next config!

This obviously deviates from the documentation, but if it works... maybe this is the way to go?

heyitsarpit commented 3 years ago

@ndimatteo Yeah, I deleted node_modules and package-lock.json and my package.json looks like this,

  "dependencies": {
    "react": "npm:@preact/compat@0.0.4",
    "react-dom": "npm:@preact/compat@0.0.4",
    "react-ssr-prepass": "npm:preact-ssr-prepass@^1.1.3"
}
teodragovic commented 3 years ago

This is what worked for me:

  "dependencies": {
    "next": "10.0.9",
    "next-plugin-preact": "^3.0.3",
    "preact": "^10.5.13",
    "preact-render-to-string": "^5.1.16",
    "react": "npm:@preact/compat@0.0.4",
    "react-dom": "npm:@preact/compat@0.0.4",
    "react-ssr-prepass": "npm:preact-ssr-prepass@^1.1.3"
  },
const withPreact = require('next-plugin-preact');

module.exports = withPreact({});

(Updated to reflect comment below)

jnv commented 3 years ago

(Just a side note, modern in experimental has been removed some time ago so it should have no influence: https://github.com/vercel/next.js/pull/19275).

MaxmaxmaximusAWS commented 3 years ago

not installing

MaxmaxmaximusAWS commented 3 years ago

use pnpm instead of npm and

"dependencies": { "next": "10.0.9", "next-plugin-preact": "^3.0.3", "preact": "^10.5.13", "preact-render-to-string": "^5.1.16", "react": "npm:@preact/compat@0.0.4", "react-dom": "npm:@preact/compat@0.0.4", "react-ssr-prepass": "npm:preact-ssr-prepass@^1.1.3" },

rtritto commented 3 years ago

Same problem here with yarn berry (v2). After I run yarn add react@npm:@preact/compat, I get:

➤ YN0000: ┌ Resolution step
➤ YN0001: │ Error: react@npm:@preact/compat isn't supported by any available resolver
    at n.getResolverByDescriptor (.yarn\releases\yarn-berry.cjs:2:337763)
    at n.bindDescriptor (.\.yarn\releases\yarn-berry.cjs:2:337128)
    at d (.\.yarn\releases\yarn-berry.cjs:2:358489)
    at async Promise.all (index 0)
    at async ie.resolveEverything (.\.yarn\releases\yarn-berry.cjs:2:359694)
    at async .\.yarn\releases\yarn-berry.cjs:2:378351
    at async f.startTimerPromise (.\.yarn\releases\yarn-berry.cjs:2:390820)
    at async ie.install (.\.yarn\releases\yarn-berry.cjs:2:378290)
    at async .\.yarn\releases\yarn-berry.cjs:2:34297
    at async Function.start (.\.yarn\releases\yarn-berry.cjs:2:389517)
➤ YN0000: └ Completed
➤ YN0000: Failed with errors in 0s 88ms

Edit 1: Resolved with yarn add react@npm:@preact/compat@*, but now I get warnings in yarn like this:

...
➤ YN0060: │ my-project@workspace:. provides react (p5a6eb) with version 0.0.4, which doesn't satisfy what next and some 
of its descendants request
...
➤ YN0002: │ next-plugin-preact@npm:3.0.4 [0f3ca] doesn't provide react (pdc818), requested by next
...

Maybe I should add a resolver in .yarnrc or somewhere?

Edit 2: With run next dev, I get:

warn  - React 17.0.1 or newer will be required to leverage all of the upcoming features in Next.js 11. Read more: https://nextjs.org/docs/messages/react-version
gu-stav commented 3 years ago

With the latest version (10.2.1) and webpack5 enabled there is a new problem:

> Build error occurred
TypeError: Cannot read property 'framework' of undefined
    at Object.webpack (/home/gustav/Development/seebruecke/frontend/node_modules/next-plugin-preact/index.js:46:27)
    at Object.webpack (/home/gustav/Development/seebruecke/frontend/node_modules/@prefresh/next/src/index.js:37:27)
    at getBaseWebpackConfig (/home/gustav/Development/seebruecke/frontend/node_modules/next/dist/build/webpack-config.js:152:412)
    at async Promise.all (index 1)
    at async Span.traceAsyncFn (/home/gustav/Development/seebruecke/frontend/node_modules/next/dist/telemetry/trace/trace.js:6:584)
    at async /home/gustav/Development/seebruecke/frontend/node_modules/next/dist/build/index.js:11:947
    at async Span.traceAsyncFn (/home/gustav/Development/seebruecke/frontend/node_modules/next/dist/telemetry/trace/trace.js:6:584)

Both work well with webpack4.

I'd suspect this line and maybe this change on the next.js side of things.

tbgse commented 3 years ago

@JoviDeCroock is this something you can look into? We briefly spoke about it already in the context of https://github.com/preactjs/next-plugin-preact/issues/25 that the latest version of next will bring some breaking changes with webpack 5, now that 10.2.1 has landed the above error is blocking us from updating the next version. I'm unable to tell if that other entry point related error with prefresh might be existing alongside this framework issue above, because we don't even get to that point.

JoviDeCroock commented 3 years ago

I'm going to try and take a look in the following days, will update the test-suite to use 10.2.1 if that doesn't surface the error I'll probably need some help reproducing

tbgse commented 3 years ago

Should be very straightforward to reproduce, for all of our repos we ran into the same issue with the update - also updated the example repo here: https://github.com/tbgse/preact-webpack5-next with the same problem. Let me know if you need more help to reproduce or test

gu-stav commented 3 years ago

@JoviDeCroock Thanks a lot. I'm also offering support and would anything test as soon as you have a PR ready.

JoviDeCroock commented 3 years ago

It seems like there's two things we need to do to fix it from a Prefresh and Next-plugin pov, first thing is that they changed cacheGroups so we'll have to do:

            if (cacheGroups && cacheGroups.framework) {
                cacheGroups.preact = Object.assign({}, cacheGroups.framework, {
                    test: preactModules
                });
                cacheGroups.commons.name = 'framework';
            } else if (cacheGroups) {
                cacheGroups.preact = {
                    name: 'commons',
                    chunks: 'all',
                    test: preactModules
                };
            }
        }

However, this does not seem like the full solution yet...

In @prefresh we'll need to resolve the entries correctly, all of a sudden there's the possibility of having undefined entries and absolute url's aren't supported in cross-dependent entries, this means that we'll have to structure our entries like this for next:

  if (typeof originalEntry === 'function') {
    return (...args) =>
      Promise.resolve(originalEntry(...args)).then((entry) => {
        const newEntry = injectEntry(entry);
        if (newEntry['react-refresh']) {
          delete newEntry['react-refresh'];
          newEntry['@prefresh/core'] = {
            import: [require.resolve('@prefresh/core')]
          }
        }

        return newEntry
      });
  }

After doing all of that Next seems to inject the DOM but not render any further which might be part of the first change

Screenshot 2021-05-20 at 12 24 22
sokra commented 3 years ago

It seems like there's two things we need to do to fix it from a Prefresh and Next-plugin pov, first thing is that they changed cacheGroups so we'll have to do:

          if (cacheGroups && cacheGroups.framework) {
              cacheGroups.preact = Object.assign({}, cacheGroups.framework, {
                  test: preactModules
              });
              cacheGroups.commons.name = 'framework';
          } else if (cacheGroups) {
              cacheGroups.preact = {
                  name: 'commons',
                  chunks: 'all',
                  test: preactModules
              };
          }
      }

next.js added a splitChunks configuration for the server bundle. next-plugin-preact should probably check isServer to only modify the cacheGroups of the client part and not the server part.

I think we can add cacheGroups: {} to next.js to avoid breaking this case.


In @prefresh we'll need to resolve the entries correctly, all of a sudden there's the possibility of having undefined entries and absolute url's aren't supported in cross-dependent entries, this means that we'll have to structure our entries like this for next:

@prefresh/webpack/src/utils/injectEntry.js doesn't handle the full range of possible webpack 5 entry configurations correctly.

These are the possible formats:

entry: "./module",
entry: ["./multiple", "./modules"],
entry: {
  name1: "./module",
  name2: ["./multiple", "./modules"],
  name3: {
    import: "./module",
    /* more options */
  },
  name4: {
    import: ["./multiple", "./modules"],
    /* more options */
  },
},
entry: () => somethingFromAbove

The advanced syntax with more options has been added for webpack 5. See here for more info: https://webpack.js.org/configuration/entry-context/#entry-descriptor

sokra commented 3 years ago

btw. if you want to inject an additional entry to all entrypoints you can do that: (only webpack 5)

const dependency = EntryPlugin.createDependency('@prefresh/core', { name: 'prefresh entry' });
compiler.hooks.make.tapAsync("Plugin", (compilation, callback) => {
  compilation.addEntry(compiler.context, dependency, undefined, callback);
});
JoviDeCroock commented 3 years ago

@sokra thank you so much for your insight, managed to get it to work. That was really insightful, will apply that EntryPlugin.createDependency measure on @prefresh/webpack as well 🙌

Fixed and published as 3.0.6

jeremylynch commented 3 years ago

I recently experienced this issue on a fresh install of nextjs 12.0.3. Problem solved by manually adding preact/compat to package.json:

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