Enable this plugin only during development mode

[jantimon]

hey

the https://github.com/jantimon/favicons-webpack-plugin is only generating the full favicon during production mode by default.
so if you set mode: "development" it will not be active unless you overrule that setting.

what about adding something similar to ReactRefreshWebpackPlugin?

the api could look like this:

// Only active during development
new ReactRefreshWebpackPlugin()

// Active for dev and prod
ReactRefreshWebpackPlugin({
  alwaysEnabled: true
})

to find out wether you are in production mode or in dev mode you can use the following snippet inside any hook:

// From https://github.com/webpack/webpack/blob/3366421f1784c449f415cda5930a8e445086f688/lib/WebpackOptionsDefaulter.js#L12-L14
    const isProductionLikeMode =
      compiler.options.mode === 'production' || !compiler.options.mode;

[pmmmwh]

hey

the https://github.com/jantimon/favicons-webpack-plugin is only generating the full favicon during production mode by default.
so if you set mode: "development" it will not be active unless you overrule that setting.

what about adding something similar to ReactRefreshWebpackPlugin?

This is already the case I believe - if you set mode to development no hooks will be attached. You can use forceEnable to override the setting though.

[jantimon]

Oh cool 👍

why would you need this code in that case to conditionally add it?

const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin');
// ... your other imports

// You can tie this to whatever mechanisms you are using to detect a development environment.
// For example, as shown here, is to tie that to `NODE_ENV` -
// Then if you run `NODE_ENV=production webpack`, the constant will be set to false.
const isDevelopment = process.env.NODE_ENV !== 'production';

module.exports = {
  // It is suggested to run the plugin in development mode only
  // If you are an advanced user and would like to setup Webpack yourselves,
  // you can also use the `none` mode,
  // but you will need to set `forceEnable: true` in the plugin options.
  mode: isDevelopment ? 'development' : 'production',
  // ... other configurations
  plugins: [
    // ... other plugins
    // You could also keep the plugin in your production config,
    // It will simply do nothing.
    isDevelopment && new ReactRefreshWebpackPlugin(),
  ].filter(Boolean),
};

[jantimon]

one more idea:

can we move all of this into the plugin?

const isDevelopment = process.env.NODE_ENV !== 'production';

module.exports = {
  // DO NOT apply the plugin in production mode!
  mode: isDevelopment ? 'development' : 'production',
  module: {
    rules: [
      // ... other rules
      {
        // for TypeScript, change the following to "\.[jt]sx?"
        test: /\.jsx?$/,
        exclude: /node_modules/,
        use: [
          // ... other loaders
          {
            loader: require.resolve('babel-loader'),
            options: {
              // ... other options
              // DO NOT apply the Babel plugin in production mode!
              plugins: [isDevelopment && require.resolve('react-refresh/babel')].filter(Boolean),
            },
          },
        ],
      },
    ],
  },
};

We could in the afterPlugins webpack hook iterate over all loaders - search for babel and check if it contains react-refresh/babel - if not push it.

So the webpack prod(disabled)/dev(enabled) config change to add the refresh plugin would look like this:

module.exports = {
  plugins: [
     new ReactRefreshWebpackPlugin()
  ]
}

That way it would even be cli compatible e.g.:

webpack --plugin react-refresh-webpack-plugin --mode production

or

webpack-dev-server --plugin react-refresh-webpack-plugin --mode development

[pmmmwh]

why would you need this code in that case to conditionally add it?

It is always good to make the configuration explicit from a library authors' standpoint, so users would know upfront if something is broken or not. Historically my experience throughout the development of this plugin is that it is more easy to break things when you don't add them conditionally - like some might forget to use Babel for some files, or included/ignored some files in the side of the plugin which they shouldn't.

The Babel plugin and the plugin both kinda smartly removes themselves if NODE_ENV is production, but is see it as an optimization rather than a heuristic users should depend on. There are many NODE_ENVs lying around too - when you run Webpack, when your app run in the browser ... What if they are not in sync?

We could in the afterPlugins webpack hook iterate over all loaders - search for babel and check if it contains react-refresh/babel - if not push it.

We could, but it would not be very resilient. For example, what should happen if the user is using an external babel.config.js file or an .babelrc file? There is no way for us to reliably detect the plugin is actually there. In another case, what if the user have multiple instances of babel-loader setup with varying configurations (e.g. electron-prerender and electron-render, non-browser environments (e.g. one for web Workers, one for the app), create-react-app where one instance processes node_modules and another processes app code)?

I am open to simplifying the configuration steps, but I don't see how we can do it unless we wrap Babel ourselves. (A corollary for this would be - what if the user is using ttypescript and a custom transform?)

[jantimon]

You are mentioning good points - the problem is that webpack configs are hard to write and even harder to understand and maintain. That's why I am always looking for ways to reduce conditions, and to decouple configuration lines.

You are right there are many many edge cases - Maybe such a feature could be behind a feature flag like autoInject or something.
That way to get started a dev could just write new ReactRefreshWebpackPlugin({ autoInject: true }).
Once they are more familiar with webpack configs and needs more control they can remove that flag at any time.

[pmmmwh]

You mentioned that Webpack configuration is difficult to maintain - and that is kinda exactly why I'm pushing back on this. The matrix of build tools and setups we have to cover makes such a flag tricky to implement and use.

I don't want to sound reluctant to change, but we have had something similar before - a check to detect whether or not the Babel plugin is applied. It just ended up throwing out false negatives too often and we've had to remove it. It is the unreliable nature that makes me uncomfortable with such a "feature" - such a nature can make bugs less discoverable, harder to diagnose and even make them impossible to fix.

This especially hurt people getting started to config Webpack - because when they rely on the flag they thought it would work wonders for them. But it might not, then this contract is broken and they now have to go look for another solution, which is to do it manually - i.e. what we've been offering.

First 3 pages of babel-loader tutorials yields at least 4 ways of configuration:

• Config in .babelrc variants
• Config in babel.config.js variants
• Config in package.json
• Config within webpack.config.js

@gaearon I would love your thoughts on this topic too.

[pmmmwh]

I also think that there are other ways to approach this problem, each with different trade-offs (e.g. performance, ergonomics)

• Add react-refresh/babel under a flag in @babel/preset-react
• Move the whole transform within Webpack's parsing run
• Apply the transform with our own loader (it currently already exists, but we don't parse the source into an AST)
• Push Babel to provide some sort of API to decide what is the configuration currently used/inject some plugins globally (difficult, especially considering babel-loader can have inline config)
• Wrap babel-loader since this is possible, but it will yield 2 Babel passes
• and ... Better docs in general maybe.

[morriq]

I came here from react-hot-loader, and installation steps discourages me since it forces my configuration to check whether is production or development build.

In react-hot-loader it's written:

The webpack patch, hot, Babel plugin, @hot-loader/react-dom etc. are all safe to use in production; they leave a minimal footprint, so there is no need to complicate your configuration based on the environment. Using the Babel plugin in production is even recommended because it switches to cleanup mode.

Please consider the same.

[pmmmwh]

I came here from react-hot-loader, and installation steps discourages me since it forces my configuration to check whether is production or development build.

In react-hot-loader it's written:

The webpack patch, hot, Babel plugin, @hot-loader/react-dom etc. are all safe to use in production; they leave a minimal footprint, so there is no need to complicate your configuration based on the environment. Using the Babel plugin in production is even recommended because it switches to cleanup mode.

Please consider the same.

The point is, there are many parts to the integration, and this plugin and the Babel plugin actually does nothing if NODE_ENV is production. Sniffing your Babel configuration is as far as I know a no-go (there are no reliable way to see what loaders are in the chain, what configurations they are using, etc.).

To ask a counter question - How should we manage your Babel configuration and make it respect Webpack's mode option?

I don't think it is necessarily a bad idea either - there is no cleanup mode for React Refresh, it's either inject or skip. So why not make it clear that they should not be used for a particular environment, if that is the intention?