Issue 5535 - Replace our module system with webpack

webpack_runner.js

Index: webpack_runner.js
diff --git a/webpack.config.js b/webpack_runner.js
similarity index 55%
rename from webpack.config.js
rename to webpack_runner.js
index 3e061af1b2a4a25c60a261bd31ba91db127f05f5..39c75a40158384a6c0bca13282f11aad74b05ab0 100644
--- a/webpack.config.js
+++ b/webpack_runner.js
@@ -16,14 +16,60 @@
  */
 
 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, INFO_PATH} = require("process").env;
+     RESOLVE_PATHS} = require("process").env;
+
+// Copied from adblockpluscore/test_runner.js
+function webpackInMemory(bundleFilename, options)
+{
+  return new Promise((resolve, reject) =>
+  {
+    // Based on this example
+    // https://webpack.js.org/api/node/#custom-file-systems
+    let memoryFS = new MemoryFS();
+
+    options.output = {filename: bundleFilename, path: "/"};
+    let webpackCompiler = webpack(options);
+    webpackCompiler.outputFileSystem = memoryFS;
 
-module.exports = {
+    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]);
+      }
+    });
+  });
+}
+
+webpackInMemory(BUNDLE_NAME, {
   context: EXTENSION_PATH,
-  devtool: "source-map",
+  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
@@ -31,7 +77,6 @@ module.exports = {
   resolve: {
     modules: RESOLVE_PATHS.split(" "),
     alias: {
-      info$: INFO_PATH,
       // 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",
@@ -71,19 +116,23 @@ module.exports = {
       }
     ]
   },
-  stats: {
-    assets: false,
-    children: false,
-    chunks: false,
-    errorDetails: true,
-    errors: true,
-    hash: false,
-    modules: false,
-    publicPath: false,
-    reasons: false,
-    source: false,
-    timings: false,
-    version: false,
-    warnings: true
+  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);
   }
-};
+);