Rebuild project on package update (#2956)

[maciej-ka]

Posting this to get early feedback.

My concerns are:

• fs.watch has few problems
• current implementation triggers duplicate rebuild after yarn add of a missing module

This is my first dive into webpack related source.
❤️ for any pieces of advice.

[maciej-ka]

In this approach, the fs.watch will be triggered, most likely, a lot of times on yarn add or yarn upgrade. Timeout here is to group these events and avoid rebuilding after a change of every file.

[viankakrisna]

why not use https://github.com/webpack/watchpack? also i think watching the whole files in node_modules would be very expensive, can we just watch all the package.jsons inside it? and ignore some folders that would be changed on runtime like .cache

[maciej-ka]

Hi, great to see you!
Webpack watch is current direction and changing a RegExp used to ignore node_modules.

[viankakrisna]

Hi, great to see you too!
Ah, I see, that would be better! would the ignored setting accept a function? I imagine it would be easier to read than a regex with lots of condition. Reading the docs it only accept regex or anymatch, but in some place like loaders' test it accept a function like (filePath) => filePath.endsWith('.js')

[maciej-ka]

I'm experimenting with changing this watch ignore definition into

  return new RegExp(
    `^(?!${escape(
      path.normalize(appSrc + '/').replace(/[\\]+/g, '/')
    )})(?!.*package\.json$).+/node_modules/`

Which, I checked in node REPL, should ignore all paths in node_modules except package.json files. Sadly, it doesn't seem to change anything in current next branch or in a custom plugin.

Still inspecting. (I started a friendship with Code Studio Node debugger to solve this puzzle)

[gaearon]

fs.watch is definitely bad, so we shouldn't use it.

I think it's a bit weird that we have this problem in the first place. I distinctly remember webpack being able to pick up changes to the installed packages. What happened?

[maciej-ka]

We'll certainly use evil.fs.watch but just hidden in dependencies: webpack -> watchpack -> chokidar which is a wrapper for fs.watch.

This happened. CRA has a webpack setup to ignore watching of node_modules. It was added to solve 293.

[maciej-ka]

Seems that webpack doesn't observe package.json files, unless they are imported.
So the plan to use webpack watcher to observe only package.json (by tweaking ignore regexp) is probably not valid.

In scenario:

1. create-react-app /tmp/demo1
2. cd demo1
3. yarn add semver-console@0
4. add import 'semver-console' into App.js
5. turn on webpack watcher on node_modules by commenting out these lines in tmp/demo1/node_modules/react-scripts/config/webpackDevServer.config.js
6. yarn start

While server is running, any change in node_modules/semver-console/index.js will trigger reload. But change in package.json will not be noticed by watcher.

Confirmed this with debugger: At this point webpack has a list of files to watch in files variable which includes .../semver-console/index.js but not a package.json file.

The observed files are calculated by visiting modules tree and collecting a list of files that are really used.

[maciej-ka]

What's next ...

a) There is a PR to bring back watching node_modules on configurations where it would not cause a slowdown.
b) It would be possible to write a webpack plugin which uses chokidar to observe just package.json files.

[petetnt]

Hey @maciej-ka!

#3982 was created from some off-the-band discussions about the issues the ignore node_modules approach introduces and how to solve them.

For example CRA now supports monorepos so we should be able to handle multitudes of node_modules. fs.watch itself isn't bad on platforms that have high-performance file watchers (ie. not OSX) and chokidar provides escape hatches on situations where polling should be enabled (virtual drives, filewatching not working for some other reason etc.) which in return kill performance on platforms where it should just play nicely.

Would love if you reviewed #3982 and told me what you think about it ^^

[maciej-ka]

3982 is so cool, because it would rebuild in both after adding and upgrading dependency. If I understand this: chokidar states that on both OSX and *nginx there will be no high CPU, however for the *nginx scenario it's advised to watch with a moderation.

Webpack plugin to watch package.json using chokidar could be a fallback. I'll update this PR.