Feat(source-filesystem): No file watchers on build

[Bouncey]

When a site has many many files coming through gatsby-source-filesystem, an issue can arise where the system running gatsby runs out of watchers. This gist has a workaround for increasing the number of watchers available.

The above fix does not work reliably in a netlify build container however. So this PR makes changes where, if the NODE_ENV is 'production', we use the built-in fs to traverse the directory tree looking for file paths. This will not attach any listeners.

If we are in 'development', we still use chokidar to monitor for file changes as per the current functionality.

[gatsbybot]

Deploy preview for using-drupal ready!

Built with commit 3699cfe

https://deploy-preview-6207--using-drupal.netlify.com

[gatsbybot]

Deploy preview for gatsbygram failed.

Built with commit 3699cfe

https://app.netlify.com/sites/gatsbygram/deploys/5b6b821bfdd72a65db274fdc

[pieh]

above 2 lines ashould be insidefileWatcher function - otherwise there would be single instance of state machine even if multiple instances of plugin are running which would cause problems

[pieh]

Others thing that comes to my mind - ignored files/directories currently isn't exactly 1:1 - it would be good to create more complex fixture and compare if both fileWatcher and fileFinder creates same nodes

[pieh]

Also worth noting - this also results in build speedup - source and transform nodes gatsbyjs.org on netlify changed from ~48s to ~38s

[m-allanson]

I'm a bit wary that having two separate implementations for finding files will be the source of subtle bugs. As an example it looks like the Windows / AppVeyor tests for this are failing at the moment.

I actually don't know how chokidar works behind the scenes, but I wonder if there's a way to use it without watchers, or to tell it to output a list of files that it will watch? A quick look at the docs seems like there's a polling method and a getWatched() method. Maybe we can make use of those instead of rolling our own file finding implementation?

[Bouncey]

@pieh The idea would be that only one would run, determined by the NODE_ENV, so the 'same node' issue shouldn't happen.

@m-allanson my understanding of chokidar is that it is a wrapper around fs.watch and anything that it finds it will attach a watcher to. My idea was to not use chokidar for the build, as the files ideally will not change during a build.

I thought I had pushed a fix for appveyor, I will try again.

[KyleAMathews]

I echo @m-allanson's concern about having two separate implementations.

But this would solve but the watchers exhaustion problem & speed up things as there's no reason to add watchers in production builds so that's wasting time.

Interesting discussion paulmillr/chokidar#410 You might want to try using one of the libraries mentioned there e.g. https://github.com/kmalakoff/readdirp-walk instead of rolling your own algorithm.

[m-allanson]

@Bouncey it'd be great to get this merged if you have a chance to try out using readdirp-walk or similar?

[thebigredgeek]

Wondering how we can push this across the finish line? This is also a blocker for internal gatsby inc stuff

[Bouncey]

Hey @m-allanson & @thebigredgeek, I have been slammed at work recently. I plan to get the changes made as soon as possible though.

[Bouncey]

Sorry for the force push, but it was a grizzly rebase. I just had a feeling I was about to add 300 commits to this bad boy.

[KyleAMathews]

Deploy preview for using-postcss-sass failed.

Built with commit 3699cfe

https://app.netlify.com/sites/using-postcss-sass/deploys/5b6b821cfdd72a65db274fe2

[KyleAMathews]

Deploy preview for image-processing failed.

Built with commit 3699cfe

https://app.netlify.com/sites/image-processing/deploys/5b6b821cfdd72a65db274fe6

[KyleAMathews]

Deploy preview for using-remark failed.

Built with commit 3699cfe

https://app.netlify.com/sites/using-remark/deploys/5b6b821efdd72a65db274fee

[thebigredgeek]

So FWIW, you can set the environment variable CHOKIDAR_USEPOLLING to a truthy value and it will bypass the watcher issue entirely, though with slightly more CPU usage.

[pieh]

I've opened issue in chokidar repo asking if disabling file watcher could be implemented directly there: paulmillr/chokidar#770 Let's see what maintainers there think about it.

[pieh]

Got response from chokidar maintainer that this isn't something that they want to support. So my best bet right now is to create abstraction that use either chokidar or readdirp depending on if we want to watch for changes or not. It seems like chokidar is using readdirp internally so hopefully same glob patterns can be used and won't cause different results - will need to investigate more.

[m-allanson]

For reference, this PR covers a similar issue #5807

[m-allanson]

I've updated this branch with the latest changes from master. There's a Windows issue that needs clearing up, I'll look into that now.

[m-allanson]

I've swapped readdirp-walk out for readdirp, as readdirp is used directly by chokidar and readdirp-walk hasn't been updated in a few years. So readdirp seems like a safer bet in terms of consistency with chokidar.

@gatsbyjs/core I think this is good for another review now.

[pieh]

Idea here:
instead of using 2 different code paths for dev and production in plugin code, what if we create hopefully thin abstraction that would have same API as chokidar, but would accept watch: true/false param (and decide to use chokidar or readdirp based on that).

That way we could do

-const watcher = chokidar.watch(pluginOptions.path, {
+const watcher = findFiles(pluginOptions.path, {
+   watch: process.env.GATSBY_EXECUTING_COMMAND === `develop`,  
    ignored: [
      `**/*.un~`,
      `**/.DS_Store`,
      `**/.gitignore`,
      `**/.npmignore`,
      `**/.babelrc`,
      `**/yarn.lock`,
      `**/bower_components`,
      `**/node_modules`,
      `../**/dist/**`,
      ...(pluginOptions.ignore || []),
    ],
  })

here as first step, and later on we could move it to its own package and do the same in gatsby core without much effort.

What are your thoughts?

[m-allanson]

Thanks @pieh, that was a good idea as it turned up some issues that I'd previously overlooked.

It looks like chokidar does some additional processing on paths before deciding which ones to ignore.

This means that for our list of default ignores to be handled correctly, we'd need to emulate chokidar functionality in Gatsby code. This seems like a great way to introduce subtle bugs into Gatsby :)

Example difference

As an example - in the default ignores we have a couple of globs like **/node_modules. The file some/dir/node_modules/packagename/subdir/foo/blah.js will be ignored by chokidar as it breaks the path down into directory fragments and runs the ignore list against all the fragments (at least that’s my understanding so far).

In our new findFiles method we use anymatch directly on the full paths. findFiles will do something like:

anymatch(`**/node_modules`, `some/dir/node_modules/packagename/subdir/foo/blah.js`)

and so won’t ignore the file, whereas chokidar will ignore the file. We could handle this case in our own code, but there may be other differences too.

Conclusion

I'm going to close this PR for now, as:

• using CHOKIDAR_USE_POLLING or increasing the number of available watchers provides a useable workaround
• writing a findFiles method that's compatible with Chokidar's ignores doesn't seem like a valuable use of time

Thanks @Bouncey for the original PR!

[m-allanson]

Additional research

I spent a bit of time looking at other projects and how they handle file watching.

This isn't particularly relevant, but I collected the info so might as well post it.

Other watchers

There's watchman from facebook, this requires an external service to be installed.

There's NSFW from Axosoft, this is a native node module and requires node-gyp to be functional before you can install it.

Other projects that watch files

VSCode seems to have a different watcher for each platform (one is chokidar).

Brunch uses chokidar’s persistent flag to switch between build and develop modes

Webpack wraps chokidar in its own watchpack library.

Parcel also wraps chokidar for its watch handling code