Split analyzer from the treemap UI code

[valscion]

This issue is a place to discuss and track a possible separation of webpack analyzer plugin code from the reporter UI code.

This idea was triggered by a proposal to add sunburst charts to the UI, where @th0r and me eventually thought it might be interesting to split these two concerns to distinct packages:

Or that the UI that is opened could be configurable, so that I could build a totally separate npm package that would only contain the UI parts that could be plugged into this packages webpack parts.

That's a very interesting idea and, at first glance, it won't be too hard to split analyzer from report UI.

We can move all current UI code from webpack-bundle-analyzer into, say, webpack-bundle-analyzer-treemap-reporter.
webpack-bundle-analyzer will be responsible only for analyzing bundle and feeding this info to reporter-plugins whose responsibility will be showing it in any way.

And the nice thing is reporter is not limited to some UI stuff - it can do anything it want with provided data!

[valscion]

I didn't hack on this on Christmas break but instead hacked on something completely different from work related matters to get a good vacation 🎅 . If I'll get to this, I'll let you know.

[valscion]

I'm gonna give this a go now. @bitpshr, would it be OK to try to get your beautiful sunburst chart to work first? It could be created as a webpack-bundle-analyzer-reporter-sunburst for example, if we'd want to try to prefix all the different reporters with webpack-bundle-analyzer-reporter- to ease discoverability.

[valscion]

Continuing from the refactoring done in #40, the current situation has a folder structure looking like this:

webpack-bundle-analyzer/
├── client
│   ├── components
│   └── vendor
├── lib
│   ├── bin
│   ├── chartData
│   └── viewer
├── node_modules
├── public
├── src
│   ├── bin
│   ├── chartData
│   └── viewer
├── test
│   ├── bundles
│   ├── src
│   └── stats
└── views

The only things reaching into top-level client, public and views folders are the root-level webpack.config.js and code under src/viewer.

Those folders and the webpack config form the implementation of the current treemap reporter.

I could extract the entire src/viewer/ folder to a new package, so that new reporters could create their own server and static strategies at will. This way all that the new reporters would need to do were to export two functions:

function generateReport(chartData, { openBrowser, reportFilename, bundleDir, logger })

and

function startServer(chartData, { openBrowser, port, logger })

The plugin's analyzerMode configuration option would select one of these. This way it would be up to the author of individual reporters to implement the static generation the way they would want to do it and to use whichever templating engine they'd like for the server build.

It might be a bit more difficult to try to extract things deeper down than these two functions, as it starts to get quite opinionated at that point and the separated reporters would be less powerful.

Take a look at the treemap implementation of these two functions:

• src/viewer/generateReport.js
• src/viewer/startServer.js

What do you think? Would this be a good split point, or should I aim for something less powerful?

[valscion]

Here's a proof of concept example showing what the code layout could look like: valscion#1

The reporter used could be customized with the CLI flag --reporter or -R, providing it either a npm package name or an absolute path to a file exporting generateReport and startServer functions as described in my post above.

Another way to customize the reporter would be to pass a module exporting those same functions as the reporter option for the plugin.

[valscion]

What do you think? Would this be a good split point, or should I aim for something less powerful?

I'll answer my own question:

I don't think this actually is a good split point, or at least not the best API.

I'd like to test out a monorepo approach for here, to really separate the reporter-treemap as its own npm package and to fine-tune the custom reporter API to be better.

For that, I'd also like to redo some build and test infrastructure currently on the repository to get better test coverage and to get a smooth build pipeline across all the packages this repository could contain.

I also wouldn't want to put many hours to this unless you would be okay with my changes, @th0r. How much space would you be willing to give me with the current infrastructure? Could I help you maintain this repository and change things around internally, targeting to a new v3 release that really would separate the reporter packages?

I really want to help and allow this plugin to work for most of the bundle related analyze use cases. To make that a reality, I would need to be able to improve the current test coverage and to make adding tests less painful than they currently are.

[th0r]

Sorry @valscion, I'm currently have very tough deadline on work so don't have time to look at it, but will do it as soon as I can!
Thanks!

[valscion]

I've got it working in our app!! 🎉 🎉

diff --git a/config/webpack/webpack.performance.config.js b/config/webpack/webpack.performance.config.js
index cffcb61c5..4cd02bfa9 100644
--- a/config/webpack/webpack.performance.config.js
+++ b/config/webpack/webpack.performance.config.js
@@ -5,7 +5,7 @@ const config = generateConfig({
   optimizeBundle: true
 });

-const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin;
+const BundleAnalyzerPlugin = require('webpack-bundle-analyzer-valscion-tmp').BundleAnalyzerPlugin;

 config.bail = true;

@@ -35,7 +35,8 @@ config.plugins = [
     // option. See more here: https://github.com/webpack/webpack/blob/webpack-1/lib/Stats.js#L21
     statsOptions: null,
     // Log level. Can be 'info', 'warn', 'error' or 'silent'.
-    logLevel: 'info'
+    logLevel: 'info',
+    reporter: require('webpack-bundle-analyzer-reporter-treemap')
   })
 ];

diff --git a/package.json b/package.json
index 284ceee4a..a0637753f 100644
--- a/package.json
+++ b/package.json
@@ -175,1 +175,2 @@
-    "webpack-bundle-analyzer": "2.1.0",
+    "webpack-bundle-analyzer-reporter-treemap": "1.0.0-alpha.1",
+    "webpack-bundle-analyzer-valscion-tmp": "3.0.0-alpha.2",

The current code is very much a proof of concept. The code can be found splitted into these:

• https://github.com/valscion/webpack-bundle-analyzer/tree/batshit-crazy — this plugin, with all reporter code removed
• https://github.com/valscion/webpack-bundle-analyzer-reporter-treemap — only the treemap reporter code

Here's the comparison view to what the #40 PR currently has: valscion/webpack-bundle-analyzer@split-reporter...valscion:batshit-crazy

I'm not yet happy on how the splitted API looks like, and I'll dive deeper into that once I get all the bits and pieces sliding into place properly.

[XVincentX]

One more point to that - would it be great if there will be any possibility to get the generated data programmatically - so that I can eventually monitor the bundle sizes in my own way.

[valscion]

Getting the generated data programmatically is one goal of this UI split to me ☺️. I'd love to be able to monitor bundle sizes as well.

[XVincentX]

Hey - I might contribute to it if you could point me to the right part to modify. I'll then figure out what to do there.

[valscion]

Thanks for the offer! I think the blocker here is me not really knowing how to progress to make this OK by @th0r, so I think it'd be best if I can figure out the split logic by myself :)

I'm still a bit unsure of how I want the API between plugin and reporter to look like. If you want to try out creating your own reporter (one that could just fetch the generated data programmatically), have a look at the split-reporter-to-new-package branch, especially the part where the BundleAnalyzerPlugin.js has changed where I use the newly split reporter package.

If you want to hack around, it would be awesome if you could create a new reporter like the treemap reporter I extracted and test out whether the split-reporter-to-new-package branch approach would work for you. All you'd need to change there would be to change this:

const reporter = require('@webpack-bundle-analyzer/reporter-treemap');

...to require your new reporter's package instead.

Your new reporter would need to expose an API similar to how @webpack-bundle-analyzer@reporter-treemap has done.

const viewer = require('./viewer');

module.exports = {
  generateReport,
  createReporter
};

function generateReport(stats, opts) {
  return viewer.generateReport(stats, opts);
}

function createReporter(initialChartData, opts) {
  const server = viewer.startServer(initialChartData, opts);
  return server.then(({ updateChartData }) => {
    return {
      updateData: newData => {
        updateChartData(newData);
      }
    };
  });
}

Basically, you'd need to implement two functions, generateReport and createReporter.

The generateReport function is called synchronously and it is only ever called once. It gets the generated chart data as its first argument, and the second argument is the options object containing information and a Logger instance you should use to log stuff to console.

// https://github.com/webpack-bundle-analyzer/reporter-treemap/blob/master/src/index.js

function generateReport(stats, opts) {
  // ...do what you want with `stats` and `opts`.
  // It doesn't matter what this function returns
}

This is how that function is called from the plugin side:

// https://github.com/th0r/webpack-bundle-analyzer/blob/split-reporter-to-new-package/src/BundleAnalyzerPlugin.js

class BundleAnalyzerPlugin {
  //
  // ...
  //
  generateStaticReport(stats) {
    const chartData = analyzer.getChartData(this.logger, stats, this.getBundleDirFromCompiler());
    // Here we call the `generateReport` function
    reporter.generateReport(chartData, {
      openBrowser: this.opts.openAnalyzer,
      reportFilename: path.resolve(this.compiler.outputPath, this.opts.reportFilename),
      bundleDir: this.getBundleDirFromCompiler(),
      logger: this.logger,
      defaultSizes: this.opts.defaultSizes
    });
  }
  //
  // ...
  //
}

The createReporter function is called when the reporter server is started, and it returns a promise. That promise should settle with an object that has updateData function in it. That updateData function will be called by the webpack plugin when new data is generated.

// https://github.com/webpack-bundle-analyzer/reporter-treemap/blob/master/src/index.js

function createReporter(initialChartData, opts) {
  // Start the server somehow...
  const server = viewer.startServer(initialChartData, opts);
  // ...and return a Promise...
  return server.then(({ updateChartData }) => {
    // ...that settles with an object that has a function in key "updateData".
    return {
      // This function here will be called by the BundleAnalyzerPlugin instance when
      // new data is available.
      updateData: newData => {
        // You probably want to do something in here with the `newData` you received.
      }
    };
  });
}

This is how that function is called from the plugin side:

// https://github.com/th0r/webpack-bundle-analyzer/blob/split-reporter-to-new-package/src/BundleAnalyzerPlugin.js

class BundleAnalyzerPlugin {
  //
  // ...
  //
  async startAnalyzerServer(stats) {
    const chartData = analyzer.getChartData(this.logger, stats, this.getBundleDirFromCompiler());
    if (this.server) {
      (await this.server).updateData(chartData);
    } else {
      this.server = reporter.createReporter(chartData, {
        openBrowser: this.opts.openAnalyzer,
        host: this.opts.analyzerHost,
        port: this.opts.analyzerPort,
        bundleDir: this.getBundleDirFromCompiler(),
        logger: this.logger,
        defaultSizes: this.opts.defaultSizes
      });
    }
  }
  //
  // ...
  //
}

I hope that the code excerpts highlight what the second options argument contains. I'm happy to shed more light to this if something is confusing.

[valscion]

Oh, and it would be very valuable if you could test this out and give feedback on what it felt like to do this and what pain points you encountered along the way.

[XVincentX]

Thanks for the notes, really valuable. I think I'll probably wait for the API to be completed/merged and released, and eventually start working on it!