Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 of 2 tasks
juterral opened this issue Jun 17, 2021 · 23 comments
Closed
1 of 2 tasks

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

juterral opened this issue Jun 17, 2021 · 23 comments

Comments

@juterral
Copy link

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

  • This is a bug
  • This is a modification request

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: quasarframework/quasar#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:

122479919-2a023d80-cfcc-11eb-947e-aa6b85432b79.mov
@alexander-akait
Copy link
Member

Please use search before create, answered #3338

@alexander-akait
Copy link
Member

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

@juterral
Copy link
Author

juterral commented Jun 17, 2021

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)

Enregistrement.de.l.ecran.2021-06-18.a.00.58.54.mov

@alexander-akait
Copy link
Member

You need set snapshot.managedPaths: []

@juterral
Copy link
Author

I already tried with this configuration too:

Capture d’écran 2021-06-18 à 01 04 54

@alexander-akait
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@juterral
Copy link
Author

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

@juterral
Copy link
Author

juterral commented Jun 17, 2021

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
Copy link
Member

alexander-akait commented Jun 17, 2021

@julien-terral found:

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

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

on 68 line

@juterral
Copy link
Author

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

@nkalinov
Copy link

nkalinov commented Mar 11, 2022

@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.

  • Tried snapshot.managedPaths: [] - no success.
  • Tried snapshot.immutablePaths: [] - no success
  • Tried watchOptions.poll: true - no success.
  • Node - Tried v14, v16 - no difference.
  • Yarn 3.2
  • webpack 5.57.0
  • webpack-dev-server 4.7.4
  • MacOS
  • core package is virtual path (it has peerDependencies).

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
Copy link
Member

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

@nkalinov
Copy link

Just to be clear, you mean that right?

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

Not working...

@alexander-akait
Copy link
Member

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

@nkalinov
Copy link

@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),
  };
};
  1. 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,
              },
            },
  1. 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
Copy link

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
Copy link

Another finding:

  • Works on Windows ✅
  • Doesn't work on MacOS ❌
  • Linux - I can't verify...

@nkalinov
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Contributor

arcanis commented Mar 16, 2022

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 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants