Code splitting support

[eight04]

If you turn on experimentalCodeSplitting, this plugin doesn't log anything. With single file input/output, the plugin works fine.

[doesdev]

Thanks for reporting that, I'll take a look at what's up when that option is given.

[doesdev]

I just wrote a couple tests that included that option. They did pass so I'm assuming the test isn't representative of the scenario you're working with.

If you want to point more towards some code, make a gist, or add / modify the tests here that would be good in aiding a fix.

If you decide to fork the repo and work with the tests some here's a couple things to note. Yarn is required as we test against several versions of Rollup. Also the tests are run in a loop over many versions but you'll want to limit tests of this feature to specific versions (I only used 0.60.x).

[eight04]

Here is the repro: https://github.com/eight04/node-test/tree/rollup-plugin-analyzer

It seems that when input has only one item, the analyzer works fine. It only happens when there are multiple entries.

[doesdev]

Thanks for the repro. Looking into it and how the hooks are called when outputting to multiple files the only way I'll be able to get it outputting something meaningful is to output separately for each file that gets written.

When outputting to a single file the hook is called once, when outputting to multiple files it's called once for each output file (which includes explicitly input files as well as chunk files). This plugin uses both the generateBundle and transformChunk hooks to correlate several data points. Given a sharper brain I may be able to come up with a way to make it output a single analysis for all outputs, but atm all I can come up with is something that outputs multiple times to the console (or to your callback if specifying onAnalysis).

I'm going to keep thinking on it, but let me know if you think it would be an acceptable experience to get a discrete output for each output file. If so I should be able to get something out tomorrow to do that.

[eight04]

I'm mostly interested in the bundle space and dependents information. They allow us to find out which module took up too much space. I think it is important to keep them ordered. The output can't be ordered when printed separately, right?

[doesdev]

They would be ordered in groups. Each analysis would correlate to an output file and would contain the analysis of only that output file's imports.

I thought there may be another hook I could use to identify when everything is bundled. If there were then I could use that to create a unified analysis of the whole thing, but unfortunately that is not the case. So it seems outputting them separately is the only option at the moment.

[doesdev]

Okay, just published 2.0.4 to address this. Albeit not in an ideal way, but unless I've overlooked something I don't think there is alternative without changes in Rollup.

There are two primary issues with this approach that I see.

1. We have multiple analysis' output, so it makes it more complicated to visually grep than if all were combined.
2. None of the output analysis are explicitly identifiable. That is to say if we have a bundle-a.js and a bundle-b.js as respective entry-points, while those will show in each analysis as children they are not identified at the top level as there is no good way to isolate what the entry point is for each call to the generateBundle hook.

The more important point for improving the experience here is the first. If all were output in one analysis there would be no need for identifying the entry-point for each output. A change that would allow for this would be an additional plugin hook that would get called only once at the end of the entire bundling process.

That being said I am hesitant to submit a PR for this to Rollup as I can't think of any other use cases for such a plugin hook. If I can put together other practical use cases that would make such a hook viable I would at least open an issue to discuss it with the Rollup team.

If you have any thoughts on any defense / use cases for such a hook or any other ideas around this let me know.

If you find what it's doing with this update to be sufficient that's cool, but I suspect it's going to be less than ideal.

[eight04]

another hook I could use to identify when everything is bundled

Isn't that the generateBundle hook?

[doesdev]

No, unfortunately that's the crux of the issue. With multiple outputs that hook is called for every output file. So within the plugin there isn't a way to know when it's done bundling everything. If there were I could collect all the module information on each generateBundle call and then when that final hook was called execute the analysis.

[eight04]

I cloned the repo and noticed that generateBundle hook is not used at all. The test result won't change if you comment out generateBundle hook. Also, runAnalysis wraps its content in a dummy promise that is resolved immediately, causing that all errors are suppressed inside the promise. When using generateBundle there is actually a type error occurred at L126 that modules is undefined.

[eight04]

Test failed on my machine (Windows 7, node v10.4.1)

``` yarn run v1.7.0 $ rollup -c && ava --verbose module.js ??index.js... ----------------------------- Rollup File Analysis ----------------------------- bundle size: 4.95 KB original size: 4.924 KB code reduction: 0 % module count: 1 ----------------------------- file: \module.js bundle space: 100 % rendered size: 4.95 KB original size: 4.924 KB code reduction: 0 % dependents: 0 ----------------------------- created index.js in 32ms ??latest: formatted returns expected string (507ms) ??latest: analyze.modules returns array (502ms) ??latest: analysis object has expected properties (501ms) ??0.60.x: formatted returns expected string (507ms) ??0.60.x: analyze.modules returns array (504ms) ??0.60.x: analysis object has expected properties (504ms) ??latest: filter with array works (558ms) ??latest: filter with string works (557ms) ??latest: filter with callback works (557ms) ??latest: root works as expected (557ms) ??0.60.x: filter with array works (551ms) ??0.60.x: filter with string works (551ms) ??0.60.x: filter with callback works (551ms) ??0.60.x: root works as expected (551ms) ??latest: limit works (576ms) ??0.60.x: limit works (569ms) ??0.50.x: formatted returns expected string (585ms) ??0.50.x: analyze.modules returns array (584ms) ??0.50.x: analysis object has expected properties (584ms) ??0.45.x: formatted returns expected string (583ms) ??0.45.x: analyze.modules returns array (581ms) ??0.45.x: analysis object has expected properties (581ms) ??0.50.x: filter with array works (589ms) ??0.50.x: filter with string works (590ms) ??0.50.x: filter with callback works (590ms) ??0.50.x: root works as expected (590ms) ??0.45.x: filter with array works (588ms) ??0.45.x: filter with string works (588ms) ??0.45.x: filter with callback works (588ms) ??0.45.x: root works as expected (588ms) ??0.50.x: plugin writes expected heading content (590ms) ??0.45.x: plugin writes expected heading content (588ms) ??0.40.x: formatted returns expected string (587ms) ??0.40.x: analyze.modules returns array (586ms) ??0.40.x: analysis object has expected properties (586ms) ??latest: it works with generated bundle as well (604ms) ??0.60.x: it works with generated bundle as well (598ms) ??0.50.x: limit works (591ms) ??0.45.x: limit works (589ms) ??0.40.x: it works with generated bundle as well (586ms) ??0.50.x: it works with generated bundle as well (605ms) ??0.45.x: it works with generated bundle as well (603ms) ??0.40.x: filter with array works (601ms) ??0.40.x: filter with string works (600ms) ??0.40.x: filter with callback works (600ms) ??0.40.x: root works as expected (600ms) ??0.40.x: limit works (604ms) ??latest: plugin writes expected heading content (622ms) ??0.60.x: plugin writes expected heading content (616ms) ??0.60.x: writes expected heading with experimentalCodeSplitting (616ms) ??0.40.x: plugin writes expected heading content (604ms) ??0.55.x: formatted returns expected string (616ms) ??0.55.x: analyze.modules returns array (613ms) ??0.55.x: analysis object has expected properties (613ms) ??0.55.x: filter with array works (619ms) ??0.55.x: filter with string works (619ms) ??0.55.x: filter with callback works (619ms) ??0.55.x: root works as expected (619ms) ??0.55.x: limit works (619ms) ??0.55.x: plugin writes expected heading content (620ms) ??0.55.x: it works with generated bundle as well (620ms) ??latest: tree shaking is accounted for (649ms) ??latest: treeshaken bundle filters with callback (648ms) ??0.60.x: tree shaking is accounted for (643ms) ??0.60.x: treeshaken bundle filters with callback (643ms) ? 0.60.x: data as expected with experimentalCodeSplitting 1 test failed 0.60.x: data as expected with experimentalCodeSplitting D:\Dev\rollup-plugin-analyzer\test\test.js:186 185: let imported = result.find((r) => r.id.indexOf(imp.id) !== -1) 186: assert.is(imported.size, imp.size) 187: }) Difference: - 8304 + 8238 imports.forEach (test/test.js:186:16) Test.fn (test/test.js:183:15) error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ```

[doesdev]

TL;DR

You're right on a number of points and your observations indicate a fix for outputting them in a single analysis. In order to make that fix though I may have to break compatibility with versions of Rollup below 0.60.0 which was only released within the last month. I'm not sure if that's something I can do, but perhaps there is a way to pull it off without breaking compatibility.

Long version

The promise thing is a (possibly unnecessary) design decision. The decision being not to block Rollup from doing it's thing, since it's an additive plugin rather than something that is necessary for bundling. That being said the decision not to at least log errors to console was a bad one and one that I'll fix.

Thanks for digging into this. Your observations did lead to a solution for this particular issue. They do open up another design choice I need to figure out.

You were correct earlier that generateBundle is called only once when the bundle is finished. This hook was added in 0.60.0 and the ongenerate and onwrite hooks were staged for deprecation (will happen in 1.0.0). I had them both in there under the false assumption that when the generateBundle hook was added Rollup was using it in place of ongenerate as the docs read that way.

The ongenerate hook is called for every output file, the generateBundle is not. Until the change I made for 2.0.4 having both would cause no adverse effects, but at present it's likely causing duplicate outputs for certain use cases.

I'll dig into this tomorrow and see what I can come up with. On the tests, is that after commenting out the generateBundle or before? Odd that it would fail as it runs against Linux in Travis and Windows on my machine so I wouldn't guess it to be an environmental difference but I suppose it could be.

[eight04]

On the tests, is that after commenting out the generateBundle or before?

Before. I'm not familiar with yarn. Maybe I have installed wrong dependencies. Commands that I used:

yarn install
yarn test

[doesdev]

Sorry for the delay on this, I'm having difficulty getting it resolved :/ Still trying to figure it out though, will let you know as I figure anything out on it.

On the test, I suppose it could be something to do with line-endings depending on how you have git setup. Testing based on output bytes isn't probably for the best due to issues like that or minor changes in Rollup potentially causing a single byte difference will result in failures. I'll find a better way to test those.

In the interim based on the output you got it is working, the failure is superficial so you can change the expected bytes in your fork to match. It's on line 182 -> size property. You may also have to change the other as well.

[doesdev]

Given this flag is deprecated and is now the default behavior and it does provide output now (but outputs more than once, which is noted in the readme) I'm going to close this. That said, if the outputting more than once deems need to open a separate issue to consolidate the output feel free to open one for that.