webpack version detection should use feature detection

[ndelangen]

the webpack version is determined using a require, but this is problematic in monorepos where the location of this plugin and the location of webpack can be unexpected.

Is there some way to infer the version we're dealing with from the compiler passed into apply?

[ndelangen]

Or resolve-from the compiler.context?

[pmmmwh]

The issue with feature detection is that we need to import stuff from Webpack that is only on a certain version. If require('webpack') does not work reliably, the imports we do (like require('webpack/lib/RuntimeModule')) would also not work reliably?

[ndelangen]

would it be possible to provide the path to webpack to the constructor?

[pmmmwh]

Hmm, that would be possible, but is it possible to describe the use cases and current limitations to me so I could understand the situation more?

[ndelangen]

I'm trying to add webpack 5 support for storybook whilst also still allowing webpack 4 support.
storybook has thus has a dependency on both, making it impossible to exactly predict where which version will be installed, on the user's system. In our monorepo, webpack 5 is installed at the root. and so is react-refresh-webpack-plugin. resulting in react-refresh-webpack-plugin always behaving as if it was ran with webpack 5, even if it's actually running webpack 4.

[phaistonian]

@ndelangen I am right there at the moment; trying to make the new storybook alpha work with react refresh.

It seems that the culprit is compilation.hooks.additionalTreeRuntimeRequirements.tap.

@pmmmwh Is there way to force using webpack 5 or 4 and override the automatic detection?

[pmmmwh]

Is there way to force using webpack 5 or 4 and override the automatic detection?

Not currently. I would be open to add that, but it would also be quite clear that you're (mostly) on your own if you opt into that.

storybook has thus has a dependency on both

Can I ask more about why not opt for resolutions or aliases (in a monorepo), or for peer dependencies on Webpack? How is the dependency chain actually configured and why would Node not resolve properly?

[phaistonian]

@pmmmwh I tried using yarn resolutions and I encountered a number of errors — not recall what those were, to be honest.

My guess is that since Storybook uses a hybrid model in order to support both Webpack 4 and 5, somehow confusion comes into play.

I will give it another try today and I come up with more context I will follow up here.

[ndelangen]

We can't enforce a single version of webpack in storybook, because we use 1 (fixed) version of webpack to build the manager, and then the user can control what version of webpack to build the preview with.

[rdsedmundo]

I'm also facing this using pnpm package manager in a monorepo. I have webpack@5 installed in the workspace root that is used for backend projects, but for a CRA app (that also uses Storybook) it's still on webpack v4 as CRA hasn't landed support for it yet.

The react-refresh plugin is trying to import the wrong version of webpack (5) and failing with:

(node:28718) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'tap' of undefined

at

compilation.hooks.additionalTreeRuntimeRequirements.tap(

I work around it by forcing pnpm to add webpack@4 as a dependency of react-refresh:

function readPackage(pkg) {
  // TODO: webpack5 https://github.com/facebook/create-react-app/issues/9994
  if (
    pkg.name === '@pmmmwh/react-refresh-webpack-plugin' &&
    pkg.peerDependencies?.webpack?.startsWith('>=4')
  ) {
    pkg.dependencies = {
      ...pkg.dependencies,
      webpack: '4.46.0',
    };
  }

  return pkg;
}

module.exports = {
  hooks: {
    readPackage,
  },
};

See also: pnpm/pnpm#3437

[pmmmwh]

I think the main problem that I'm unsure here is how should we pull in files when require does not work reliably.

It is quite easy to make the plugin to detect current version on the compiler, but the difference between Webpack 4 and Webpack 5 is larger that we would need to require Webpack 5 specific files, which means we are again dependent on require working properly. One solution mentioned above would be to allow you provide the path to Webpack, but it would make things quite complicated and I'm not sure if that's the best approach.

[ndelangen]

Webpack should dependency inject webpack into the Plugin.apply call, maybe it already does?

[rdsedmundo]

What are the drawbacks of doing dynamic requires as needed? Like this (pseudocode):

let moduleInfo;

if (compiler.version === 5) {
  moduleInfo = require('webpack/lib/moduleInfo');
} else if (compiler.version === 4) {
  const compilation = require('webpack/lib/compilation');
  moduleInfo = compilation.stats.modules.info;
}

moduleInfo.forEach(...)

I see that you're already doing this in some places, and if you say that it's easy to detect webpack's version from the compiler object, I'm not sure if I got what is blocking us here then.

[ndelangen]

Consider this package getting hoised to the top of a monorepo,
Webpack 5 is also hoisted to the top.

Now a sub-package in this monorepo depends on webpack 4 and runs it, and uses this plugin.

Webpack 4 is adding this addon, which will require webpack (will require webpack 5, because it's hoisted).
Now webpack 4 throws an error, because the plugin uses webpack 5 api's.

---

You can simply not assume that the webpack version you require is the same as the webpack version your plugin is invoked with.

[pmmmwh]

I'm not sure if I got what is blocking us here then.

The issue is the require does not work reliably - even if I version detect correctly from the compiler instance, requiring Webpack 5 specific paths will still throw with file not found.

[rdsedmundo]

Now I understand, thank you all for the explanations. I assume this is a quite common scenario for loaders and plugins, we could see what others are doing. I'm not sure if they rely on require or if they pick what they need from the compiler object. One scenario that I've seen in the past is to use separate branches and major versions in NPM for supporting different versions of webpack.

@alexander-akait @sokra by any chance could you give any tips for us here? TLDR: we have multiple versions of webpack in the node_modules from a monorepo, and even though we call the plugin using webpack@4, inside the plugin code require('webpack') returns webpack@5, which confuses the plugin and makes it use features that aren't available on the webpack version it was called from.

[alexander-akait]

For webpack v5 please use compier.webpack https://github.com/webpack/webpack/blob/master/types.d.ts#L1814 to get webpack exports, for v4 compatibility you can use const webpack = compier.webpack || require('webpack');

[pmmmwh]

TIL I can do compiler.webpack.

I guess this solves the issue of having WP4 hoisted and wanted to use WP5, but if the hoisted version is WP5 and the local version is WP4 this would sill break as there is a require call ...

I'll try to test this out.

[alexander-akait]

compiler.webpack fix the problem when webpack v4 on top, this is a more common problem, than versa vice. Also in future to avoid this problem better always use compiler.webpack, we migrate on this in all official plugins and loaders

[pmmmwh]

Yep I think this would solve most if not all of our problems. Thanks for chiming in!

Is there any lower-bound version for WP5 to use compiler.webpack?

[alexander-akait]

5.1, but I think using const webpack = compiler.webpack || require('webpack'); solves the problems, in future dropping webpack v4 solve the whole problem

[ndelangen]

in future dropping webpack v4 solve the whole problem

For a while, there will always be new versions in the future, and the hoisting problem will be there always.

[alexander-akait]

Also if you use webpack-cli, we have WEBPACK_PACKAGE env variable (https://github.com/webpack/webpack-cli/blob/master/packages/webpack-cli/lib/webpack-cli.js#L12), so you can set webpack package location

[pmmmwh]

Is there a way to access JavascriptParserHelpers? It doesn't seem to be available under the compiler.

Edit: Hmm, never mind, tapping through DefinePlugin seems to work as well.

[pmmmwh]

Hi @ndelangen @rdsedmundo -

I've implemented proper version detection and pass-through imports on the compiler instance for Webpack 5 in #415. Would you mind testing it out in your projects?

npm install -D -S git://github.com/pmmmwh/react-refresh-webpack-plugin.git#feat/feature-detection
yarn add -D git://github.com/pmmmwh/react-refresh-webpack-plugin.git#feat/feature-detection
pnpm add -D git://github.com/pmmmwh/react-refresh-webpack-plugin.git#feat/feature-detection

I'll merge that once I got around to fill out tests for the changes and we can confirm that the PR works in reality.

[rdsedmundo]

It worked for my project. Thanks for working on that! 🎉