Issue 5535 - Replace our module system with webpack

webpack_runner.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+"use strict";
+
 const path = require("path");
 const process = require("process");
 
 const MemoryFS = require("memory-fs");
 const webpack = require("webpack");
 
-let {EXTENSION_PATH, ENTRY_POINTS, OUTPUT_PATH, BUNDLE_NAME,
-     RESOLVE_PATHS} = require("process").env;
+// We read the configuration from STDIN rather than as an argument to improve
+// the output on error. Otherwise the (fairly huge) configuration is printed
+// along with the actual error message.
+let inputChunks = [];
+process.stdin.setEncoding("utf-8");
+process.stdin.on("data", chunk => { inputChunks.push(chunk); });
+process.stdin.on("end", () =>
+{
+  let {bundles, extension_path,
+       info_module, resolve_paths} = JSON.parse(inputChunks.join(""));
 
-// Copied from adblockpluscore/test_runner.js
-function webpackInMemory(bundleFilename, options)
-{
-  return new Promise((resolve, reject) =>
+  // The contents of the info module is passed to us as a string from the Python
+  // packager and we pass it through to our custom loader now so it is available
+  // at bundle time.
+  require("./info-loader").setInfoModule(info_module);
+
+  // Since the cost of starting Node.js and loading all the modules is hugely
+  // larger than actually producing bundles we avoid paying it multiple times,
+  // instead producing all the bundles in one go.
+  let options = [];
+  for (let {bundle_name, entry_points} of bundles)
   {
-    // Based on this example
-    // https://webpack.js.org/api/node/#custom-file-systems
-    let memoryFS = new MemoryFS();
+    options.push({
+      context: extension_path,
+      devtool: "source-map",
+      module: {
+        rules: [{
+          include: path.join(__dirname, "info.js"),
+          use: ["info-loader"]
+        }]
+      },
+      entry: entry_points,
+      output: {
+        path: "/",
+        filename: bundle_name
+      },
+      resolve: {
+        modules: resolve_paths,
+        alias: {
+          // To use our custom loader for the info module we must first set up
+          // an alias to a file that exists.
+          info$: path.join(__dirname, "info.js"),
+          // Prevent builtin Node.js modules from being used instead of our own
+          // when the names clash. Once relative paths are used this won't be
+          // necessary.
+          url$: "url.js",
+          events$: "events.js",
+          punycode$: "punycode.js"
+        },
+        plugins: [
+          function()
+          {
+            // Our old module system in packagerChrome.py used to prefix
+            // module names with the name of their parent directory and an
+            // underscore - but only when that directory wasn't called
+            // "lib". This plugin is for backwards compatability, but can
+            // be removed once use of that deprecated syntax has been
+            // replaced.
+            this.plugin("described-resolve", (request, callback) =>
+            {
+              let target = request.request;
 
-    options.output = {filename: bundleFilename, path: "/"};
-    let webpackCompiler = webpack(options);
-    webpackCompiler.outputFileSystem = memoryFS;
+              let prefixIndex = target.indexOf("_");
+              if (prefixIndex == -1)
+                return callback();
 
-    webpackCompiler.run((err, stats) =>
-    {
-      // Error handling is based on this example
-      // https://webpack.js.org/api/node/#error-handling
-      if (err)
-      {
-        let reason = err.stack || err;
-        if (err.details)
-          reason += "\n" + err.details;
-        reject(reason);
-      }
-      else if (stats.hasErrors())
-        reject(stats.toJson().errors);
-      else
-      {
-        let bundle = memoryFS.readFileSync("/" + bundleFilename, "utf-8");
-        memoryFS.unlinkSync("/" + bundleFilename);
-        resolve([bundle, stats]);
+              let prefix = target.substring(0, prefixIndex);
+              if (prefix == "lib")
+                return callback();
+
+              let fixedTarget = path.join(prefix,
+                                           target.substring(prefixIndex + 1));
+              return this.doResolve(
+                "resolve", Object.assign({}, request, {request: fixedTarget}),
+                "Changed prefixed path using legacy buildtools syntax from " +
+                target + " to " + fixedTarget,
+                callback
+              );
+            });
+          }
+        ]
+      },
+      resolveLoader: {
+        modules: [path.resolve(__dirname)]
       }
     });
+  }
+
+  // Based on this example
+  // https://webpack.js.org/api/node/#custom-file-systems
+  let memoryFS = new MemoryFS();
+  let webpackCompiler = webpack(options);
+
+  webpackCompiler.outputFileSystem = memoryFS;
+  webpackCompiler.run((err, stats) =>
+  {
+    // Error handling is based on this example
+    // https://webpack.js.org/api/node/#error-handling
+    if (err)
+    {
+      let reason = err.stack || err;
+      if (err.details)
+        reason += "\n" + err.details;
+      throw new Error(reason);
+    }
+    else if (stats.hasErrors())
+      throw new Error(stats.toJson().errors.join("\n"));
+    else
+    {
+      let files = {};
+
+      for (let config of options)
+      {
+        let filepath = path.join(config.output.path, config.output.filename);
+        let mappath = filepath + ".map";
+        files[filepath] = memoryFS.readFileSync(filepath, "utf-8");
+        files[mappath] = memoryFS.readFileSync(mappath, "utf-8");
+      }
+
+      console.log(JSON.stringify(files));
+    }
   });
-}
-
-webpackInMemory(BUNDLE_NAME, {
-  context: EXTENSION_PATH,
-  devtool: "inline-source-map",
-  entry: ENTRY_POINTS.split(" "),
-  module: {
-    rules: [{
-      include: path.join(EXTENSION_PATH, "lib", "info.js"),

[Wladimir Palant, 2017/10/04 10:38:07]

This looks like we are supposed to do require("./info"). That would be
inconsistent however, with the file info.js not actually existing in the
directory. It would make more sense if require("info") would work, this is
really a platform module. If `test: /^info$/` won't do the job, then a resolve
alias should do. The alias should also make the rule here unnecessary, you can
specify "info-loader!info.json" as explicit alias target.

[kzar, 2017/10/04 13:13:25]

I originally tried `test: /^info$/` but it turned out the resolved absolute path
for the module was used. Then I did something like `test: /info.js$` but I
realised that would also match other modules which were called (or had a name
ending in) info.js. So in the end I went with the absolute path, anything which
webpack resolved to the placeholder file would be replaced, but nothing else.

I've tried using an alias like you suggested instead but I can't get it to work,
when using an absolute or relative path.

[Wladimir Palant, 2017/10/09 10:54:54]

I tried this as well and it seems that module resolution happens before the
loader has a chance - so the file has to exist. However, aliases can be used to
map it to anything, and mapping it to info-loader.js itself works. Here is the
configuration that worked for me:

    module: {
      rules: [{
        include: path.join(__dirname, "info-loader.js"),
        use: ["info-loader"]
      }]
    },
    resolve: {
      alias: {
        info$: path.join(__dirname, "info-loader.js"),
      },
    },
    resolveLoader: {
      modules: [path.resolve(__dirname)]
    }

[kzar, 2017/10/09 13:52:14]

Cool idea, Done.

[kzar, 2017/10/10 13:51:04]

I've just noticed a down-side to this approach, it messes up the source maps.
The info module is listed under buildtools/lib/info-loader.js instead of
lib/info.js.

[Wladimir Palant, 2017/10/10 16:33:15]

Well, if we really want it to show up as info.js we could add a zero-sized
info.js file to buildtools. Not sure it's worth it however.

[kzar, 2017/10/10 17:03:38]

Done.

-      use: ["info-loader"]
-    }]
-  },
-  output: {
-    path: OUTPUT_PATH,
-    filename: BUNDLE_NAME
-  },
-  resolve: {
-    modules: RESOLVE_PATHS.split(" "),
-    alias: {
-      // Prevent builtin Node.js modules from being used instead of our own when
-      // the names clash. Once relative paths are used this won't be necessary.
-      url$: "url.js",
-      events$: "events.js",
-      punycode$: "punycode.js"
-    },
-    plugins: [
-      function()
-      {
-        // Our old module system in packagerChrome.py used to prefix
-        // module names with the name of their parent directory and an
-        // underscore - but only when that directory wasn't called
-        // "lib". This plugin is for backwards compatability, but can
-        // be removed once use of that deprecated syntax has been
-        // replaced.
-        this.plugin("described-resolve", (request, callback) =>
-        {
-          let target = request.request;
-
-          let prefix_index = target.indexOf("_");
-          if (prefix_index == -1)
-            return callback();
-
-          let prefix = target.substring(0, prefix_index);
-          if (prefix == "lib")
-            return callback();
-
-          let fixed_target = path.join(prefix,
-                                       target.substring(prefix_index + 1));
-          return this.doResolve(
-            "resolve", Object.assign({}, request, {request: fixed_target}),
-            "Changed prefixed path using legacy buildtools syntax from " +
-            target + " to " + fixed_target,
-            callback
-          );
-        });
-      }
-    ]
-  },
-  resolveLoader: {
-    modules: [path.resolve(__dirname)]
-  }
-}).then(
-  ([bundle, stats]) =>
-  {
-    console.log(bundle);
-  },
-  e =>

[Wladimir Palant, 2017/10/04 10:38:07]

I prefer a separate .catch() callback, it makes code more obvious - particularly
with multi-line callbacks.

[kzar, 2017/10/04 13:13:25]

Done.

-  {
-    // I would prefer to simply leave the exception unhandled, since we want the
-    // script to end in that case. Unfortunatley though Node.js displays a
-    // deprecation warning if we do:
-    //   "[DEP0018] DeprecationWarning: Unhandled promise rejections are
-    //    deprecated. In the future, promise rejections that are not handled
-    //    will terminate the Node.js process with a non-zero exit code."

[Wladimir Palant, 2017/10/04 10:38:07]

Well, Node correctly objects to unhandled rejections, is it really worth stating
here?

[kzar, 2017/10/04 13:13:25]

Done.

-    console.error(e.join("\n"));
-    process.exit(1);
-  }
-);
+});