webpack / webpack-dev-server

Serves a webpack app. Updates the browser on changes. Documentation https://webpack.js.org/configuration/dev-server/.
MIT License
7.8k stars 1.44k forks source link

HMR not working properly when modifying files in node_modules folder #3453

Closed julien-terral closed 3 years ago

julien-terral commented 3 years ago

OS: Tried on MacOS, Windows 10, CentOS & Debian10 webpack version: 5.37.1 webpack-dev-server: 4.0.0-beta.3

When I edit my files in the src directory, the live reload happens with the latest modifications.

However when editing my dependency files (in node_modules) for dev purposes, the console warned me that the webpack-dev-server hot reloads but my components don't re-render and all my changes are not taken into account (even if I refresh the whole page). I also tried with npm link or yarn link, but issue was the same.

With webpack 4, I never encountered this issue. I was able to change any file in the node_modules folder and had hot reload working (without any specific configuration).

Code

I've tried to add this config to the devServer options:

devServer: {
    https: false,
    port: 8080,
    open: false,
    watchOptions: {
      ignored: [
        'node_modules',
        '!node_modules/my-module',
      ],
    },
  },

and even this one, without success:

devServer: {
    https: false,
    port: 8080,
    open: false,
    watchFiles: 'node_modules/*',
  },

This is what's displayed in the console when editing a file in a node_modules package (hot reload not working):

121386308-2de9ec00-c94a-11eb-9f54-5b78c5157883

When editing an src file (hot reload working):

121387733-33940180-c94b-11eb-9084-c910c5e8086a

I thought the issue was related to Quasar, and opened a ticket but it doesn't seem to be the case. One of the creators suggested me to open a ticket here. (Link to the Quasar issue: https://github.com/quasarframework/quasar/issues/9626)

I checked on your GitHub closed issues and found some similar issues but unfortunately none of the answers did the trick for me.

Expected Behavior

When writing a console.log or an alert code in a node_modulesfile, changes are detected and the screen is automatically updated.

Actual Behavior

A change is detected and a new build is made, but the console says "[webpack-dev-server] App hot update..." Also, the screen is not updated. After ending the process, you must restart it for the changes to take effect.

For Bugs; How can we reproduce the behavior?

Link to the reproducing repo: https://github.com/julien-terral/webpack-5-hmr-issue

Video:

https://user-images.githubusercontent.com/27774317/122480157-9b41f080-cfcc-11eb-93ac-f231475958ed.mov

alexander-akait commented 3 years ago

Please use search before create, answered https://github.com/webpack/webpack-dev-server/issues/3338

alexander-akait commented 3 years ago

For watchFiles, you need to use node_modules/**/*, because watchFiles: 'node_modules/*' watch only top

julien-terral commented 3 years ago

Hi @alexander-akait, I mentioned in my issue that I already found the #3338 answer but it's not working for me.

I tried watchFiles : 'node_modules/*/', it reloads the page but my components aren't update. (cf. the new video attached)

https://user-images.githubusercontent.com/27774317/122482285-87988900-cfd0-11eb-9417-6676c7907a18.mov

alexander-akait commented 3 years ago

You need set snapshot.managedPaths: []

julien-terral commented 3 years ago

I already tried with this configuration too:

Capture d’écran 2021-06-18 à 01 04 54
alexander-akait commented 3 years ago

You need set this option here https://github.com/julien-terral/webpack-5-hmr-issue/blob/main/quasar.conf.js#L68, it is not quasar option

alexander-akait commented 3 years ago

Looks chain is not supported snapshot, but if you open node_modules/webpack/lib/config/defaults.js on 319 line (where we set default value for snapshot.managedPaths) and change code on:

A(snapshot, "managedPaths", () => {
  return [];
});

all works fine

alexander-akait commented 3 years ago

Give me couple minutes I will look at workaround for chain-webpack

julien-terral commented 3 years ago

I'll try that, thank you for your time you're awesome!

julien-terral commented 3 years ago

I confirm that it works like a charm with the fix that you provided me (when I'm update the node_modules/webpack/lib/config/defaults.js file). If you can find a workaround for chain-webpack it'll be perfect!

alexander-akait commented 3 years ago

@julien-terral found:

chainWebpack (chain) {
  chain.plugin('eslint-webpack-plugin')
    .use(ESLintPlugin, [{ extensions: [ 'js', 'vue' ] }])

  chain.merge({
    snapshot: {
      managedPaths: []
    }
  })
},

on 68 line

julien-terral commented 3 years ago

Thank you so much for your time @alexander-akait, it will help me a lot, I was looking for a workaround for days! 🙏

nkalinov commented 2 years ago

@alexander-akait We're using Yarn PnP workspaces with a few packages. We have a core package that is used by other packages and it's transpiled on the fly by adding its path to babel-loader's include prop.

No issues with Webpack4 but after the recent upgrade to WP5 changes to the core package stopped being detected from the largest app?! Yup, change detection works fine from smaller apps and it also works fine if I comment out one of the big chunks part of this large app.

I know it would be ideal to have a repro but I hope you see given the "size" factor it's not something easy to do. Any ideas? Anything related to limits on number of files being watched?

Could it be related to Yarn somehow? cc @arcanis

Thanks!

alexander-akait commented 2 years ago

Try to set it on [] https://github.com/webpack/webpack/blob/main/lib/config/defaults.js#L416, i.e. immutablePaths

nkalinov commented 2 years ago

Just to be clear, you mean that right?

  snapshot: {
    managedPaths: [],
    immutablePaths: [],
  },

Not working...

alexander-akait commented 2 years ago

@nkalinov Can you provide small example with steps, no need show code, just example of configuration to reproduce the problem, thanks

nkalinov commented 2 years ago

@alexander-akait Thanks for following up! Okay, firstly the relevant parts of the monorepo are as follows:

./packages/
  |- babel-preset-react
  |- react-scripts
  |- core
  |- app1
  |- app2...
  1. babel-preset-react is a very thin wrapper of babel-preset-react-app + common babel plugins/presets in one place:

    module.exports = (api, opts = {}) => {
    const env = api.env();
    const isEnvDevelopment = env === 'development';
    
    if (opts.useFastRefresh == null) {
    opts.useFastRefresh = true;
    }
    
    return {
    presets: [[require('babel-preset-react-app'), opts]],
    plugins: [
      isEnvDevelopment &&
        opts.useFastRefresh &&
        require.resolve('react-refresh/babel'),
      [require('@babel/plugin-proposal-decorators'), { legacy: true }],
    ].filter(Boolean),
    };
    };
  2. react-scripts is a fork of react-scripts@5 - everything within the package is the same as the official except for small tweaks to webpack.config notably:

    2.1. JS-related module.rules. Included paths.core and paths.reactSpring to transpilation as well as a few libraries to add IE11 support.

            // Process application JS with Babel.
            // The preset includes JSX, Flow, TypeScript, and some ESnext features.
            {
              test: /\.(js|mjs|jsx|ts|tsx)$/,
              include: [paths.appSrc, paths.core, paths.reactSpring],
              loader: require.resolve('babel-loader'),
              options: {
                customize: require.resolve(
                  'babel-preset-react-app/webpack-overrides'
                ),
    
                // This is a feature of `babel-loader` for webpack (not Babel itself).
                // It enables caching results in ./node_modules/.cache/babel-loader/
                // directory for faster rebuilds.
                cacheDirectory: true,
                // See #6846 for context on why cacheCompression is disabled
                cacheCompression: false,
                compact: isEnvProduction,
              },
            },
            // Process any JS outside of the app with Babel.
            {
              test: /\.(js|mjs)$/,
              exclude: path => {
                // Excluded paths
                const excludedPaths = [
                  /@babel(?:\/|\\{1,2})runtime/,
                  paths.appSrc,
                ];
                if (
                  excludedPaths.some(obj =>
                    typeof obj === 'string'
                      ? path.includes(obj)
                      : obj.test(path)
                  )
                ) {
                  return true;
                  // return false;
                }
    
                // Transpile IE11(mostly) third-party libs.
                const includedPaths = [
                  // query-string@^7 deps
                  'query-string',
                  'split-on-first',
                  'strict-uri-encode',
                  // end query-string@^7 deps
                  'yup',
                  'd3-[a-z^]*',
                  '@opentelemetry-[a-z^]*',
                  '@pmmmwh/react-refresh-webpack-plugin',
                  '@splunk/otel-web',
                  'react-spring',
                  'dayjs-business-days',
                ].join('|');
    
                const supportRegex = new RegExp(includedPaths, 'i');
    
                if (supportRegex.test(path)) return false;
    
                return /\.yarn/.test(path) || /node_modules/.test(path);
              },
              loader: require.resolve('babel-loader'),
              options: {
                compact: false,
                cacheDirectory: true,
                // See #6846 for context on why cacheCompression is disabled
                cacheCompression: false,
                // Babel sourcemaps are needed for debugging into node_modules
                // code.  Without the options below, debuggers like VSCode
                // show incorrect code and set breakpoints on the wrong lines.
                sourceMaps: shouldUseSourceMap,
                inputSourceMap: shouldUseSourceMap,
              },
            },
  3. app1, app2 : 3.1. Depend on core package: "dependencies": { ... "core": "workspace:packages/core", ... } 3.1. Depend on forked react-scripts package: "devDependencies": { ... "react-scripts": "workspace:packages/react-scripts", ... } 3.2. Extend forked babel preset via their root babel.config.js (no additional plugins).

app1 works but app2 doesn't 😕

Interesting observation 1: In main App.js file we have all our pages split as separate chunks:

const Page1 = React.lazy(() => import('./Page1'));
... + around 50 more chunks

If I comment out half of them, recompilation works!

Interesting observation 2: Follow up to the first observation - after the initial build, if I put back all chunks (so remove the comments) the recompilation still works. (Which suggests not a file limit issue maybe?)

Meanwhile I also tried sudo launchctl limit maxfiles 1000000 1000000 - no effect.

nkalinov commented 2 years ago

Hi @alexander-akait , I've added a simple Webpack plugin to tap into the watcher-related hooks to dig for more info. There are some weird things going on...

All the plugin does is:

    compiler.hooks.invalid.tap('MyExampleWebpackPlugin', (file, callback) => {
      console.log(chalk.red('compiler.hooks.invalid'), file);
    });

    compiler.hooks.watchRun.tapAsync(
      'MyExampleWebpackPlugin',
      (compilation, callback) => {
        console.log(chalk.white('compiler.hooks.watchRun'));
        console.log(
          _.pick(compilation, [
            'watchFileSystem',
            'modifiedFiles',
            'removedFiles',
            'assetsInfo',
          ])
        );

        callback();
      }
    );

Here are the logs: watchRun-working.txt is when it works (by commenting out some of the split chunks), watchRun-broken.txt is when it doesn't work.

@simon-core-virtual is the core package.

  1. Notice how on the initial build there are 3 recompilations happening - is that expected? (Search for "compiler.hooks.watchRun"). What's up with all those removedFiles triggering them?

  2. Also, at the last recompilation in watchRun-working notice how a compiler.hooks.invalid is emitted right before the actual recompilation of the modified file?

nkalinov commented 2 years ago

Another finding:

nkalinov commented 2 years ago

Switched to nodeLinker: node-modules and everything is working so I think I'll disable PnP for now... 😞 I guess we can pass the ball to @arcanis ... any ideas? (Using Yarn 3.2)

alexander-akait commented 2 years ago

oh, sorry for delay, yes, pnp has problem here, but based on different result on different os, it can be bug in pnp code, do you use the latest version?

nkalinov commented 2 years ago

Yes, we are on Yarn 3.2. I can also try the latest master ("from sources") but I don't see anything related to that in their CHANGELOG. Anyways, will report tomorrow. Meanwhile I hope @arcanis would have a minute to give it a thought.

arcanis commented 2 years ago

It's difficult to say since there is no repro. Given that it works with X dependencies but not Y I'd tend to think it doesn't come from either Webpack or Yarn (otherwise I'd expect it to always fail), but that's just a guess 🤔