Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move chart data calculation to source code edges #81

Merged
merged 10 commits into from
Jul 14, 2017

Conversation

valscion
Copy link
Member

@valscion valscion commented May 27, 2017

The goal of this PR was to get src/viewer.js to a place where it no longer requires any other internal module.

This PR supercedes #40 with a much less scary refactoring. The only potentially breaking change is the undocumented usage of require('webpack-bundle-analyzer').start export. As it was never part of the documented API, I thought it was safe to remove instead of figuring out how to still support it.

I moved client/ and views/ root level directories, that were only a concern for the reporter UI, inside reporter/ root level directory. This also moved the build-time UI artefacts from public/ to reporter/public/, cleaning up the root level folder structure a bit:

screen shot 2017-05-27 at 12 43 21


I will follow-up with another PR where I will extract the src/viewer.js to a completely new package, that will be created under @webpack-bundle-analyzer npm organization, and I'll name the package as @webpack-bundle-analyzer/reporter-treemap.

This new package will be installed as a dependency of webpack-bundle-analyzer so that everything will still function as before. I want the first step of cutting out the reporter to be invisible to users of this package, so that we can release a minor version upgrade without breaking any users and have less things to worry about when the reporter package can be customized.

More on these later.

We'll figure this out later, and use a monorepo approach :)

@valscion
Copy link
Member Author

Please review this @th0r soon so we won't have merge conflicts in the future. The changes in here should be safe for all the documented use cases.

@valscion
Copy link
Member Author

I noticed that I had forgotten to update the files array in package.json, so the next publish would've failed.

I fixed that, and checked that the contents of npm pack look reasonable to me:

$ tar -tvf webpack-bundle-analyzer-2.8.2.tgz
-rw-r--r--  0 501    20       2429 May 27 16:27 package/package.json
-rw-r--r--  0 501    20       4235 May 26 14:14 package/README.md
-rw-r--r--  0 501    20       1093 Jan 21 21:00 package/LICENSE
-rw-r--r--  0 501    20       4129 May 27 12:54 package/CHANGELOG.md
-rw-r--r--  0 501    20       5328 May 27 16:29 package/lib/BundleAnalyzerPlugin.js
-rw-r--r--  0 501    20       3711 May 27 16:29 package/lib/Logger.js
-rw-r--r--  0 501    20       5484 May 27 16:29 package/lib/analyzer.js
-rw-r--r--  0 501    20        263 May 27 16:29 package/lib/index.js
-rw-r--r--  0 501    20       6107 May 27 16:29 package/lib/parseUtils.js
-rw-r--r--  0 501    20       8956 May 27 16:29 package/lib/tree.js
-rw-r--r--  0 501    20       4630 May 27 16:29 package/lib/viewer.js
-rwxr-xr-x  0 501    20       4414 May 27 16:29 package/lib/bin/analyzer.js
-rw-r--r--  0 501    20     176710 May 27 16:29 package/reporter/public/viewer.js
-rw-r--r--  0 501    20     726613 May 27 16:29 package/reporter/public/viewer.js.map
-rw-r--r--  0 501    20        181 May 27 16:27 package/reporter/views/script.ejs
-rw-r--r--  0 501    20        416 May 27 16:27 package/reporter/views/viewer.ejs
-rw-r--r--  0 501    20       3215 May 27 16:27 package/src/BundleAnalyzerPlugin.js
-rw-r--r--  0 501    20        938 Jan 21 21:00 package/src/Logger.js
-rw-r--r--  0 501    20       4562 May 27 12:38 package/src/analyzer.js
-rw-r--r--  0 501    20        249 May 27 12:39 package/src/index.js
-rw-r--r--  0 501    20       6313 May 26 14:14 package/src/parseUtils.js
-rw-r--r--  0 501    20       4948 May 26 14:14 package/src/tree.js
-rw-r--r--  0 501    20       3162 May 27 16:27 package/src/viewer.js
-rwxr-xr-x  0 501    20       3405 May 27 16:27 package/src/bin/analyzer.js

@valscion
Copy link
Member Author

valscion commented May 27, 2017

If you want only the reporter splitting, you can also just check out #82 which contains the commits in here as well. ☺️

@valscion valscion changed the base branch from master to version-3 July 14, 2017 11:03
@valscion
Copy link
Member Author

I created a new major feature branch, version-3, where I intend to push pull requests one by one. I want to keep these PRs focused but to not merge to master until we think version 3 is ready.

@valscion valscion merged commit 318f588 into version-3 Jul 14, 2017
@valscion valscion deleted the split-reporter-2 branch July 14, 2017 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant