From 480bdf1bc74669064bd20f7d026af255950cb951 Mon Sep 17 00:00:00 2001
From: William Chargin <wchargin@gmail.com>
Date: Tue, 31 Jul 2018 15:27:32 -0700
Subject: [PATCH] Refine the build directory cleaning logic (#577)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
We were asking the `clean-webpack-plugin` to remove the `build/`
directory in all cases. However, Webpack accepts a command-line
parameter `--output-path`. When such a parameter is passed, we would be
removing the wrong directory.

The proper behavior is to remove “whatever the actual output path is”.
Webpack exposes this information, but it appears that the
`clean-webpack-plugin` does not take advantage of it. Therefore, this
commit includes a small Webpack plugin to do the right thing.

Test Plan:
Test that the behavior is correct when no output directory is specified:
```
mkdir -p build && touch build/wat && yarn build && ! [ -e build/wat ]
```

Test that the behavior is correct with an explicit `--output-path`:
```
outdir="$(mktemp -d)" && touch "${outdir}/wat" && \
    yarn build --output-path "${outdir}" && \
    ! [ -e "${outdir}/wat" ]
```

Test that the plugin refuses to remove the root directory:

```
! yarn build --output-path . && \
    sed -i '/path: /d' config/makeWebpackConfig.js && ! yarn build
```

(Feel free to comment out the actual `rimraf.sync` line in the plugin
when testing this.)

wchargin-branch: clean-actual-build-directory
---
 config/RemoveBuildDirectoryPlugin.js | 39 ++++++++++++++++++++++++++++
 config/makeWebpackConfig.js          |  4 +--
 package.json                         |  1 +
 yarn.lock                            |  2 +-
 4 files changed, 43 insertions(+), 3 deletions(-)
 create mode 100644 config/RemoveBuildDirectoryPlugin.js

diff --git a/config/RemoveBuildDirectoryPlugin.js b/config/RemoveBuildDirectoryPlugin.js
new file mode 100644
index 0000000..944e7d3
--- /dev/null
+++ b/config/RemoveBuildDirectoryPlugin.js
@@ -0,0 +1,39 @@
+// @flow
+
+const fs = require("fs");
+const path = require("path");
+const rimraf = require("rimraf");
+
+// Note: the following type-import just resolves to `any`.
+/*:: import type {Compiler} from "webpack"; */
+
+module.exports = class RemoveBuildDirectoryPlugin {
+  apply(compiler /*: Compiler */) {
+    if (compiler.hooks) {
+      console.warn(
+        "" +
+          "You appear to be running Webpack >= 4. " +
+          "The RemoveBuildDirectoryPlugin should be forward-compatible, " +
+          "but you should update it to use the new APIs. See " +
+          "<https://github.com/webpack/webpack/releases/tag/v4.0.0-beta.0> " +
+          "for details."
+      );
+    }
+    compiler.plugin("compile", () => {
+      const outputPath = compiler.options.output.path;
+      // If a build config has no `output.path` property, and no
+      // `--output-path` is passed on the command line, then Webpack
+      // will default to building into the current directory. Removing
+      // the whole Git repository would be mighty rude, so we protect
+      // against that case.
+      if (fs.existsSync(path.join(outputPath, ".git"))) {
+        throw new Error(
+          "Refusing to remove build directory with a Git repository: " +
+            outputPath
+        );
+      }
+      console.warn("Removing build directory: " + outputPath);
+      rimraf.sync(outputPath);
+    });
+  }
+};
diff --git a/config/makeWebpackConfig.js b/config/makeWebpackConfig.js
index 288cb18..b33d272 100644
--- a/config/makeWebpackConfig.js
+++ b/config/makeWebpackConfig.js
@@ -4,7 +4,7 @@ const express = require("express");
 const os = require("os");
 const path = require("path");
 const webpack = require("webpack");
-const CleanPlugin = require("clean-webpack-plugin");
+const RemoveBuildDirectoryPlugin = require("./RemoveBuildDirectoryPlugin");
 const ManifestPlugin = require("webpack-manifest-plugin");
 const SWPrecacheWebpackPlugin = require("sw-precache-webpack-plugin");
 const StaticSiteGeneratorPlugin = require("static-site-generator-webpack-plugin");
@@ -259,7 +259,7 @@ function plugins(mode /*: "development" | "production" */) {
   ];
   const prodOnlyPlugins = [
     // Remove the output directory before starting the build.
-    new CleanPlugin([paths.appBuild], {root: paths.root}),
+    new RemoveBuildDirectoryPlugin(),
     // Minify the code.
     new webpack.optimize.UglifyJsPlugin({
       compress: {
diff --git a/package.json b/package.json
index d9dabaf..3ee4e09 100644
--- a/package.json
+++ b/package.json
@@ -60,6 +60,7 @@
     "prettier": "^1.13.4",
     "raf": "3.4.0",
     "react-dev-utils": "^5.0.0",
+    "rimraf": "^2.6.2",
     "static-site-generator-webpack-plugin": "^3.4.1",
     "style-loader": "0.19.0",
     "sw-precache-webpack-plugin": "0.11.4",
diff --git a/yarn.lock b/yarn.lock
index 8c963ac..f03395e 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -6749,7 +6749,7 @@ right-align@^0.1.1:
   dependencies:
     align-text "^0.1.1"
 
-rimraf@2, rimraf@^2.2.8, rimraf@^2.5.1, rimraf@^2.5.4, rimraf@^2.6.1:
+rimraf@2, rimraf@^2.2.8, rimraf@^2.5.1, rimraf@^2.5.4, rimraf@^2.6.1, rimraf@^2.6.2:
   version "2.6.2"
   resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.6.2.tgz#2ed8150d24a16ea8651e6d6ef0f47c4158ce7a36"
   dependencies: