Refactor code to allow splitting bundle data extraction from UI code

[valscion]

The goal of this PR is to split the UI code into a separate package that can be leveraged by the current webpack-bundle-analyzer code.

See #32 for more reasoning

[valscion]

So far I have moved files around src/ directory so that I could get a better focus on what files require each other. I hope that these file locations were internal to this module and weren't part of the public API.

Each commit up to this point has passed tests, which can be seen from https://travis-ci.org/valscion/webpack-bundle-analyzer/builds, too

I was wondering, why do some files require other modules via the lib/ build directory? I understand why the tests might want to do that, but not why the source files do that.

I'll change all the non-test requires to point to files inside src directory. It should be easy-ish to revert, if there is a good reason to still require from lib.

[valscion]

I've first had to refactor the current code to get it to a state where I feel more confident in splitting out distinct parts. I try to keep the steps small so that following along one commit a time should make for a pleasant review experience.

Please let me know if you have trouble understanding anything I've done. I'm so far targeting to create breathing room for the separation, and I'm not yet 100% sure what the code will look like once I'm done.

[valscion]

EDIT:

Let's ignore this comment for now, we don't need to address it in this PR

---

At this point in the refactoring, I'm starting to miss the lovely flow types as all the various option parameter defaults and plumbing is starting to get confusing again.

What is your stance on static typing, @th0r? Would you be fine if I'd introduce some gradual typing to the webpack-bundle-analyzer codebase so that touching some function parameters wouldn't so easily break accidentally?

[valscion]

Huh? Why did the 640098b commit build pass? https://travis-ci.org/th0r/webpack-bundle-analyzer/builds/194015131

It should've been broken! It referenced a file that got moved to a different folder:

// test/viewer/parseUtils.js
const { parseBundle } = require('../../lib/viewer/parseUtils');

😕 It should've been this:

const { parseBundle } = require('../../lib/chartData/parseUtils');

Really strange.

EDIT: Oh, mocha didn't pick up those tests at all as they were inside a subdirectory.... oh well. d86ea96

[valscion]

What's the reason behind not requiring bundleDir to be always present, but having this check in src/viewer.js?

      if (!path.isAbsolute(reportFilepath)) {
        reportFilepath = path.resolve(bundleDir || process.cwd(), reportFilepath);
      }

I see you also check for its presence in src/analyzer.js module's getViewerData function:

  // Trying to parse bundle assets and get real module sizes if `bundleDir` is provided
  let parsedModuleSizes = null;
  let bundlesSources = {};
  let parsedModules = {};

  if (bundleDir) {

The process.cwd() defaulting seems to have been first introduced in a commit adding "static report" mode and the protective if(bundleDir) in getViewerData has been there ever since the initial commit.

The bundleDir value can come in via only two ways, that are more easily spotted in this PR:

1. From the plugin instance getting it from this.compiler.outputPath
2. From the CLI arguments or inferring it from the bundle stat file location

It seems like there shouldn't be any scenario where the bundleDir should be null or undefined after these steps. If I haven't misunderstood, the this.compiler.outputPath comes from output.path webpack config value which in turn is required by webpack to work.

So it should be fairly safe to remove the overly protective check from the code. It would make my life a bit easier as I'd have one less thing to worry about 😄

[valscion]

I went and removed the bundleDir optionality in the last commit. As the indentation changed, it might be best viewed with whitespace turned off using the special ?w=1 query parameter in the URL: 21385bf?w=1

[valscion]

I'll tackle the actual splitting of UI and chart data extraction in a new PR so that we can focus on only the refactoring here. Otherwise this PR risks becoming too big and the review would get waaaaay too cumbersome.

Let me know if you have any questions! I'm very open in discussing any approaches I've taken. If you've got critique, I want to hear it! 😄

[valscion]

Hiya @th0r, have you had any time to look into this? If you're having trouble maintaining this project, I could help you. It'd be a shame for this project to wither as it is so useful to many :)

[valscion]

Hi @th0r, would you be able to review this? It'd be nice to get this refactor merged so that there would be less merge conflicts in the future.

I'm fairly confident that this does not introduce any changes or backwards-incompatibilities. webpack-bundle-analyzer has never seemed to support reaching to the insides of this plugin, so moving things around should also be safe.

I would like to continue figuring out a good approach on splitting the view code altogether, but that I would do in another place :)

The current file hierarchy looks like this now:

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.

With this in place I have a much better situation with figuring out how to split the reporter away from the data generation.

[valscion]

I'll close this in favor of #81 that features a much smaller amount of code changes.