Skip to content

WebpackDevServer: (Only) Ignore node_modules if polling is required #3982

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions packages/react-dev-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,32 @@ const publicPath = config.output.publicPath;
printHostingInstructions(appPackage, publicUrl, publicPath, 'build', true);
```

#### `checkShouldIgnoreNodeModules(): boolean`

Checks if `node_modules` should be ignored from file watching.<br>
Excessive polling can cause CPU overloads on some systems (see https://github.com/facebookincubator/create-react-app/issues/293), so we use two heuristics to determine if `chokidar` file watching is going to use polling:

- The state of the `process.env.CHOKIDAR_USEPOLLING` environment variable
- The existence of `fsevents` module on OSX.

If the environment variable is truthy, or `fsevent` either doesn't exist or it fails to load, polling is enforced and we
we ignore node_modules.

```js
const WebpackDevServer = require('webpack-dev-server');
const shouldIgnoreNodeModules = require('react-dev-utils/checkShouldIgnoreNodeModules');
const watchOptions = {};

if (shouldIgnoreNodeModules) {
watchOptions.ignored = /node_modules;
}

const compiler = /**/
const server = new WebpackDevServer(compiler, {
watchOptions,
});
```

#### `WebpackDevServerUtils`

##### `choosePort(host: string, defaultPort: number): Promise<number | null>`
Expand Down
5 changes: 0 additions & 5 deletions packages/react-dev-utils/__tests__/.eslintrc

This file was deleted.

55 changes: 0 additions & 55 deletions packages/react-dev-utils/__tests__/ignoredFiles.test.js

This file was deleted.

20 changes: 0 additions & 20 deletions packages/react-dev-utils/ignoredFiles.js

This file was deleted.

5 changes: 1 addition & 4 deletions packages/react-dev-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
"FileSizeReporter.js",
"formatWebpackMessages.js",
"getProcessForPort.js",
"ignoredFiles.js",
"inquirer.js",
"InterpolateHtmlPlugin.js",
"launchEditor.js",
Expand All @@ -31,6 +30,7 @@
"openChrome.applescript",
"printBuildError.js",
"printHostingInstructions.js",
"shouldIgnoreNodeModules.js",
"WatchMissingNodeModulesPlugin.js",
"WebpackDevServerUtils.js",
"webpackHotDevClient.js",
Expand Down Expand Up @@ -62,8 +62,5 @@
},
"devDependencies": {
"jest": "22.1.2"
},
"scripts": {
"test": "jest"
}
}
60 changes: 60 additions & 0 deletions packages/react-dev-utils/shouldIgnoreNodeModules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* Copyright (c) 2018-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

// While Create React App does not use polling for watching files by default,
// polling can occur if CHOKIDAR_USEPOLLING environment variable is
// set to a truthy value. Excessive polling can cause CPU overloads
// on some systems (see https://github.com/facebookincubator/create-react-app/issues/293),
// which is why we ignore node_modules if polling is enforced.
let shouldIgnoreNodeModules = false;

const envForceUsePolling = process.env.CHOKIDAR_USEPOLLING;

if (envForceUsePolling) {
const usePollingLower = envForceUsePolling.toLowerCase();

// Chokidar rules https://github.com/paulmillr/chokidar/blob/master/index.js#L99
if (
usePollingLower === 'true' ||
usePollingLower === '1' ||
!!usePollingLower
) {
shouldIgnoreNodeModules = true;
}
}

if (process.platform === 'darwin') {
// On OSX polling can also occur if chokidar fails to load the
// `fsevents` module. Check if the module is loaded:
const cacheKeys = Object.keys(require.cache);
const cachedFsEvents =
require.cache[cacheKeys.find(i => i.includes('/fsevents/fsevents.js'))];

if (cachedFsEvents && cachedFsEvents.loaded) {
// fsEvents is loaded, check if its WebpackDevServers
let parentIsWebpackDevServer = false;
let parent = cachedFsEvents.parent;

while (parent && !parentIsWebpackDevServer) {
if (parent.id.includes('webpack-dev-server')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super happy about this in particular. In the future we'll migrate off WDS. Do we need to hardcode this at all?
Can you explain more about why this check is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like mentioned somewhere about, the check is most likely not necessary, but anyway: consider case where ModuleX and WebpackDevServer both load Chokidar, and due to something ModuleX properly loads fsevents but WDS fails to do so -> WDS polls eventhough it didn't successfully load the module.

That said,

  • I don't know if that's even possible 🤔
  • Nothing else in the CRA stack currently depends on Chokidar, so the check is currently rather moot anyway.

I am totally okay with removing it if you are 🆗

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it WDS require-ing fsevents, or webpack? I would assume that webpack should be somewhere on the stack because WDS doesn't (afaik) do any watching manually, it just tells webpack to watch.

I'd be fine to change this line if it referenced just webpack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly its WDS that requires chokidar to which the watchOptions are passed in WDS's own implementation of something similar to watchpack which eventually requires fsevents.

parentIsWebpackDevServer = true;
}

parent = parent.parent;
}

if (!parentIsWebpackDevServer) {
shouldIgnoreNodeModules = true;
}
} else {
shouldIgnoreNodeModules = true;
}
}

module.exports = shouldIgnoreNodeModules;
16 changes: 8 additions & 8 deletions packages/react-scripts/config/webpackDevServer.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@

const errorOverlayMiddleware = require('react-dev-utils/errorOverlayMiddleware');
const noopServiceWorkerMiddleware = require('react-dev-utils/noopServiceWorkerMiddleware');
const ignoredFiles = require('react-dev-utils/ignoredFiles');
const shouldIgnoreNodeModules = require('react-dev-utils/shouldIgnoreNodeModules');
const config = require('./webpack.config.dev');
const paths = require('./paths');

const protocol = process.env.HTTPS === 'true' ? 'https' : 'http';
const host = process.env.HOST || '0.0.0.0';

const watchOptions = {};

if (shouldIgnoreNodeModules) {
watchOptions.ignored = /node_modules/;
}

module.exports = function(proxy, allowedHost) {
return {
// WebpackDevServer 2.4.3 introduced a security fix that prevents remote
Expand Down Expand Up @@ -71,13 +77,7 @@ module.exports = function(proxy, allowedHost) {
// WebpackDevServer is noisy by default so we emit custom message instead
// by listening to the compiler events with `compiler.plugin` calls above.
quiet: true,
// Reportedly, this avoids CPU overload on some systems.
// https://github.com/facebook/create-react-app/issues/293
// src/node_modules is not ignored to support absolute imports
// https://github.com/facebook/create-react-app/issues/1065
watchOptions: {
ignored: ignoredFiles(paths.appSrc),
},
watchOptions,
// Enable HTTPS if the HTTPS environment variable is set to 'true'
https: protocol === 'https',
host: host,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-scripts/template/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2490,7 +2490,7 @@ HTTPS | :white_check_mark: | :x: | When set to `true`, Create React App will run
PUBLIC_URL | :x: | :white_check_mark: | Create React App assumes your application is hosted at the serving web server's root or a subpath as specified in [`package.json` (`homepage`)](#building-for-relative-paths). Normally, Create React App ignores the hostname. You may use this variable to force assets to be referenced verbatim to the url you provide (hostname included). This may be particularly useful when using a CDN to host your application.
CI | :large_orange_diamond: | :white_check_mark: | When set to `true`, Create React App treats warnings as failures in the build. It also makes the test runner non-watching. Most CIs set this flag by default.
REACT_EDITOR | :white_check_mark: | :x: | When an app crashes in development, you will see an error overlay with clickable stack trace. When you click on it, Create React App will try to determine the editor you are using based on currently running processes, and open the relevant source file. You can [send a pull request to detect your editor of choice](https://github.com/facebook/create-react-app/issues/2636). Setting this environment variable overrides the automatic detection. If you do it, make sure your systems [PATH](https://en.wikipedia.org/wiki/PATH_(variable)) environment variable points to your editor’s bin folder. You can also set it to `none` to disable it completely.
CHOKIDAR_USEPOLLING | :white_check_mark: | :x: | When set to `true`, the watcher runs in polling mode, as necessary inside a VM. Use this option if `npm start` isn't detecting changes.
CHOKIDAR_USEPOLLING | :white_check_mark: | :x: | When set to `true`, the watcher runs in polling mode, as necessary inside a VM. Use this option if `npm start` isn't detecting changes. Note that setting this option to `true` disables `node_modules` watching due to the resource intensive nature of the operation.
GENERATE_SOURCEMAP | :x: | :white_check_mark: | When set to `false`, source maps are not generated for a production build. This solves OOM issues on some smaller machines.
NODE_PATH | :white_check_mark: | :white_check_mark: | Same as [`NODE_PATH` in Node.js](https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders), but only relative folders are allowed. Can be handy for emulating a monorepo setup by setting `NODE_PATH=src`.

Expand Down
4 changes: 0 additions & 4 deletions tasks/e2e-simple.sh
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ if [ $APPVEYOR != 'True' ]; then
fi
cd ../..

cd packages/react-dev-utils/
yarn test
cd ../..

cd packages/confusing-browser-globals/
yarn test
cd ../..
Expand Down