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

Rebuild project on package update (#2956) #3936

Closed
wants to merge 1 commit 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
33 changes: 33 additions & 0 deletions packages/react-dev-utils/WatchChangedNodeModulesPlugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* 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.
*/

// This Webpack plugin ensures that any change in node_modules package.json forces a project rebuild.

'use strict';

var fs = require('fs');

var timeout;

class WatchChangedNodeModulesPlugin {
constructor(nodeModulesPath) {
this.nodeModulesPath = nodeModulesPath;
}

apply(compiler) {
fs.watch(
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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')

this.nodeModulesPath,
{ persistent: false, recursive: true },
() => {
clearTimeout(timeout);
timeout = setTimeout(() => compiler.run(() => {}), 1000);
Copy link
Contributor Author

@maciej-ka maciej-ka Jan 29, 2018

Choose a reason for hiding this comment

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

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.

}
);
}
}

module.exports = WatchChangedNodeModulesPlugin;
1 change: 1 addition & 0 deletions packages/react-dev-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"openChrome.applescript",
"printHostingInstructions.js",
"WatchMissingNodeModulesPlugin.js",
"WatchChangedNodeModulesPlugin.js",
"WebpackDevServerUtils.js",
"webpackHotDevClient.js"
],
Expand Down
8 changes: 7 additions & 1 deletion packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin');
const CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin');
const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin');
const WatchChangedNodeModulesPlugin = require('react-dev-utils/WatchChangedNodeModulesPlugin');
const eslintFormatter = require('react-dev-utils/eslintFormatter');
const ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin');
const getClientEnvironment = require('./env');
Expand Down Expand Up @@ -333,11 +334,16 @@ module.exports = {
// a plugin that prints an error when you attempt to do this.
// See https://github.com/facebook/create-react-app/issues/240
new CaseSensitivePathsPlugin(),
// If you require a missing module and then `npm install` it, you still have
// If you require a missing module and then `yarn add` it, you still have
// to restart the development server for Webpack to discover it. This plugin
// makes the discovery automatic so you don't have to restart.
// See https://github.com/facebook/create-react-app/issues/186
new WatchMissingNodeModulesPlugin(paths.appNodeModules),
// If you upgrade package version, you still have to restart the development
// server for Webpack to discover it. This plugin makes the discovery automatic
// so you don't have to restart.
// and https://github.com/facebook/create-react-app/issues/2956
new WatchChangedNodeModulesPlugin(paths.appNodeModules),
// Moment.js is an extremely popular library that bundles large locale files
// by default due to how Webpack interprets its code. This is a practical
// solution that requires the user to opt into importing specific locales.
Expand Down