From 5ddcfe228fa5cdf2727c1a1df7664bbe64b759d8 Mon Sep 17 00:00:00 2001 From: petetnt Date: Mon, 5 Feb 2018 23:31:39 +0200 Subject: [PATCH 1/5] Only ignore node_modules if polling is enabled Signed-off-by: petetnt --- packages/react-dev-utils/__tests__/.eslintrc | 5 -- .../__tests__/ignoredFiles.test.js | 55 ------------------- packages/react-dev-utils/ignoredFiles.js | 20 ------- packages/react-dev-utils/package.json | 4 -- .../config/webpackDevServer.config.js | 20 ++++--- packages/react-scripts/template/README.md | 2 +- 6 files changed, 13 insertions(+), 93 deletions(-) delete mode 100644 packages/react-dev-utils/__tests__/.eslintrc delete mode 100644 packages/react-dev-utils/__tests__/ignoredFiles.test.js delete mode 100644 packages/react-dev-utils/ignoredFiles.js diff --git a/packages/react-dev-utils/__tests__/.eslintrc b/packages/react-dev-utils/__tests__/.eslintrc deleted file mode 100644 index 55f121d152d..00000000000 --- a/packages/react-dev-utils/__tests__/.eslintrc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "env": { - "jest": true - } -} diff --git a/packages/react-dev-utils/__tests__/ignoredFiles.test.js b/packages/react-dev-utils/__tests__/ignoredFiles.test.js deleted file mode 100644 index 6feed979797..00000000000 --- a/packages/react-dev-utils/__tests__/ignoredFiles.test.js +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Copyright (c) 2015-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'; - -const ignoredFiles = require('../ignoredFiles'); - -describe('ignore watch files regex', () => { - it('normal file', () => { - const appSrc = '/root/src/'; - const isIgnored = ignoredFiles(appSrc).test('/foo'); - const isIgnoredInSrc = ignoredFiles(appSrc).test('/root/src/foo'); - - expect(isIgnored).toBe(false); - expect(isIgnoredInSrc).toBe(false); - }); - - it('node modules', () => { - const appSrc = '/root/src/'; - const isIgnored = ignoredFiles(appSrc).test('/root/node_modules/foo'); - - expect(isIgnored).toBe(true); - }); - - it('node modules inside source directory', () => { - const appSrc = '/root/src/'; - const isIgnored = ignoredFiles(appSrc).test('/root/src/node_modules/foo'); - const isIgnoredMoreThanOneLevel = ignoredFiles(appSrc).test( - '/root/src/bar/node_modules/foo' - ); - - expect(isIgnored).toBe(false); - expect(isIgnoredMoreThanOneLevel).toBe(false); - }); - - it('path contains source directory', () => { - const appSrc = '/root/src/'; - const isIgnored = ignoredFiles(appSrc).test( - '/bar/root/src/node_modules/foo' - ); - - expect(isIgnored).toBe(true); - }); - - it('path starts with source directory', () => { - const appSrc = '/root/src/'; - const isIgnored = ignoredFiles(appSrc).test('/root/src2/node_modules/foo'); - - expect(isIgnored).toBe(true); - }); -}); diff --git a/packages/react-dev-utils/ignoredFiles.js b/packages/react-dev-utils/ignoredFiles.js deleted file mode 100644 index 73a6e8bc5cf..00000000000 --- a/packages/react-dev-utils/ignoredFiles.js +++ /dev/null @@ -1,20 +0,0 @@ -/** - * Copyright (c) 2015-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'; - -const path = require('path'); -const escape = require('escape-string-regexp'); - -module.exports = function ignoredFiles(appSrc) { - return new RegExp( - `^(?!${escape( - path.normalize(appSrc + '/').replace(/[\\]+/g, '/') - )}).+/node_modules/`, - 'g' - ); -}; diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index ca08b4e612b..7ed2d1b4bda 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -20,7 +20,6 @@ "FileSizeReporter.js", "formatWebpackMessages.js", "getProcessForPort.js", - "ignoredFiles.js", "inquirer.js", "InterpolateHtmlPlugin.js", "launchEditor.js", @@ -62,8 +61,5 @@ }, "devDependencies": { "jest": "22.1.2" - }, - "scripts": { - "test": "jest" } } diff --git a/packages/react-scripts/config/webpackDevServer.config.js b/packages/react-scripts/config/webpackDevServer.config.js index 74119d27f39..735636bb282 100644 --- a/packages/react-scripts/config/webpackDevServer.config.js +++ b/packages/react-scripts/config/webpackDevServer.config.js @@ -10,13 +10,23 @@ const errorOverlayMiddleware = require('react-dev-utils/errorOverlayMiddleware'); const noopServiceWorkerMiddleware = require('react-dev-utils/noopServiceWorkerMiddleware'); -const ignoredFiles = require('react-dev-utils/ignoredFiles'); 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 = {}; + +// 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. +if (process.env.CHOKIDAR_USEPOLLING) { + watchOptions.ignored = /node_modules/; +} + module.exports = function(proxy, allowedHost) { return { // WebpackDevServer 2.4.3 introduced a security fix that prevents remote @@ -71,13 +81,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, diff --git a/packages/react-scripts/template/README.md b/packages/react-scripts/template/README.md index 2c533d49687..052678303fb 100644 --- a/packages/react-scripts/template/README.md +++ b/packages/react-scripts/template/README.md @@ -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`. From b32b4a09be1dafbd36e6b98551a3aef5dd1ae0df Mon Sep 17 00:00:00 2001 From: petetnt Date: Mon, 5 Feb 2018 23:38:31 +0200 Subject: [PATCH 2/5] Unify the USEPOLLING check to Chokidar rules Signed-off-by: petetnt --- .../config/webpackDevServer.config.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/react-scripts/config/webpackDevServer.config.js b/packages/react-scripts/config/webpackDevServer.config.js index 735636bb282..08f5dc0eca1 100644 --- a/packages/react-scripts/config/webpackDevServer.config.js +++ b/packages/react-scripts/config/webpackDevServer.config.js @@ -23,8 +23,19 @@ const watchOptions = {}; // 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. -if (process.env.CHOKIDAR_USEPOLLING) { - watchOptions.ignored = /node_modules/; +const usePolling = process.env.CHOKIDAR_USEPOLLING; + +if (usePolling) { + const usePollingLower = usePolling.toLowerCase(); + + // Chokidar rules https://github.com/paulmillr/chokidar/blob/master/index.js#L99 + if ( + usePollingLower === 'true' || + usePollingLower === '1' || + !!usePollingLower + ) { + watchOptions.ignored = /node_modules/; + } } module.exports = function(proxy, allowedHost) { From 058472f9a4667ddf5ae98ce597490deef12445c0 Mon Sep 17 00:00:00 2001 From: petetnt Date: Wed, 7 Feb 2018 00:08:10 +0200 Subject: [PATCH 3/5] Add OSX fsevents check, move the checks to react-dev-utils Signed-off-by: petetnt --- packages/react-dev-utils/README.md | 26 ++++++++ .../shouldIgnoreNodeModules.js | 60 +++++++++++++++++++ .../config/webpackDevServer.config.js | 21 +------ 3 files changed, 89 insertions(+), 18 deletions(-) create mode 100644 packages/react-dev-utils/shouldIgnoreNodeModules.js diff --git a/packages/react-dev-utils/README.md b/packages/react-dev-utils/README.md index 5279ade8f41..e05780ed0a9 100644 --- a/packages/react-dev-utils/README.md +++ b/packages/react-dev-utils/README.md @@ -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.
+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` diff --git a/packages/react-dev-utils/shouldIgnoreNodeModules.js b/packages/react-dev-utils/shouldIgnoreNodeModules.js new file mode 100644 index 00000000000..bb4d2088dea --- /dev/null +++ b/packages/react-dev-utils/shouldIgnoreNodeModules.js @@ -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')) { + parentIsWebpackDevServer = true; + } + + parent = parent.parent; + } + + if (!parentIsWebpackDevServer) { + shouldIgnoreNodeModules = true; + } + } else { + shouldIgnoreNodeModules = true; + } +} + +module.exports = shouldIgnoreNodeModules; diff --git a/packages/react-scripts/config/webpackDevServer.config.js b/packages/react-scripts/config/webpackDevServer.config.js index 08f5dc0eca1..b5f19e8b58a 100644 --- a/packages/react-scripts/config/webpackDevServer.config.js +++ b/packages/react-scripts/config/webpackDevServer.config.js @@ -10,6 +10,7 @@ const errorOverlayMiddleware = require('react-dev-utils/errorOverlayMiddleware'); const noopServiceWorkerMiddleware = require('react-dev-utils/noopServiceWorkerMiddleware'); +const shouldIgnoreNodeModules = require('react-dev-utils/shouldIgnoreNodeModules'); const config = require('./webpack.config.dev'); const paths = require('./paths'); @@ -18,24 +19,8 @@ const host = process.env.HOST || '0.0.0.0'; const watchOptions = {}; -// 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. -const usePolling = process.env.CHOKIDAR_USEPOLLING; - -if (usePolling) { - const usePollingLower = usePolling.toLowerCase(); - - // Chokidar rules https://github.com/paulmillr/chokidar/blob/master/index.js#L99 - if ( - usePollingLower === 'true' || - usePollingLower === '1' || - !!usePollingLower - ) { - watchOptions.ignored = /node_modules/; - } +if (shouldIgnoreNodeModules) { + watchOptions.ignored = /node_modules/; } module.exports = function(proxy, allowedHost) { From 3a1e25105601e431b72bbbe069edffc06c813ce2 Mon Sep 17 00:00:00 2001 From: petetnt Date: Wed, 7 Feb 2018 00:11:00 +0200 Subject: [PATCH 4/5] Currently no tests in react-dev-utils Signed-off-by: petetnt --- tasks/e2e-simple.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 37e168dd7d2..3ae71fcb4c9 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -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 ../.. From 0394d014e9c33defef1ef6f50e9287eb1dc3831e Mon Sep 17 00:00:00 2001 From: petetnt Date: Mon, 12 Feb 2018 15:13:30 +0200 Subject: [PATCH 5/5] Add shouldIgnoreNodeModules to files in package.json Signed-off-by: petetnt --- packages/react-dev-utils/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index 7ed2d1b4bda..5465651d897 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -30,6 +30,7 @@ "openChrome.applescript", "printBuildError.js", "printHostingInstructions.js", + "shouldIgnoreNodeModules.js", "WatchMissingNodeModulesPlugin.js", "WebpackDevServerUtils.js", "webpackHotDevClient.js",