List modules in a consistent order

[maaarghk]

Fixes #23

[maaarghk]

@fredden Testing out an alternative approach to #120 - it doesn't require creating a new context or re-loading everything so it's a bit faster, for me it does give consistent results, but different than on the other MR for various reasons. I just used onResourceLoad, despite this warning in the comments of the previous PR:

I was able to detect dependencies the first time something loaded, but not when a second script required the same

A long time has passed but I'm not seeing this at all against M2.4.3. But, I've only tested with bundling and minification both disabled so maybe that has something to do with it?

The dependencyMaps generated here seem more complete. Comparing the dependencyMaps from #120, I note that in that PR knockoutjs/knockout or domReady do not get mapped as a dependency of anything. In the end it's not accurate because knockoutjs/knockout does go into the common bundle and is passed as a dep when (for example) knockout-repeat is defined - probably not noticed last time since anything that ships with magento comes after it alphabetically :) but I can see this causing a problem if somebody has a third party module where the only dep is knockout, but which comes before knockout alphabetically. I think the issue has something to do with aliases configured in map not being copied to the new context, which is why this approach not requiring a new context feels a bit more sane.

I also moved the code which re-ordered the dependency map into a new function orderModules which just takes objects as inputs and is not aware of any page context. That way we should be able to write some tests for that function - to make sure it will always output a working order, that it will deal with circular deps in a general way (I removed the whitelist as I didn't see any examples), maybe shuffling the inputs about and checking the order is always correct.

Would be cool if you could run this against the same site and data you were using on #120, perhaps we could converge on something. For me, I don't think #120 can be merged whilst there's an unknown known issue with circular deps, or when the dependency map generation doesn't handle aliases.

[fredden]

@maaarghk thanks for looking into this. I like the idea of separating out the different components; this will make it easier to add tests going forward. When I worked on #120, I was trying to keep the number of changed files to a minimum.

I also like the idea of using onResourceLoad to get the list of modules as the page loads, or refining how we determine when the page has 'finished' so we can extract a list of modules.

Thanks for the code comments. These help a lot when reviewing and will be useful with future maintenance.

Are you seeing consistent results when you run this multiple times? When I run ./cli.js generate --cms-url https://demo-m2.bird.eu/ --category-url https://demo-m2.bird.eu/gear/bags.html --product-url https://demo-m2.bird.eu/gear/bags/compete-track-tote.html --skip-checkout, I sometimes get different content in magepack.config.js. The order makes sense, but not all the same modules are listed (which does impact the order, but consistently / as expected).

From what I can tell, this does produce a consistent load order / result on the websites I've tested this on. I have some minor code style comments, but on the whole this looks good to me. :+1:

[maaarghk]

Agreed with all style suggestions and updated accordingly,

Are you seeing consistent results when you run this multiple times?

I was against a local dev copy of my clients' sites, but didn't try any others yet - I'll try to find some time next week to test it against the site you suggested to see if I can work out why you might not be seeing consistent results. Could be the timeout is just a bit too flakey, I'll see if there's some way to wait for requests to finish too. Something that occurred to me for example is on slow sites, the shipping estimate ajax on /checkout/cart might not finish on time, which might cause any text elements required for that wee panel not to load.