search

Kitware / candela

Visualization components for the web
https://candela.readthedocs.io
Apache License 2.0
116 stars 29 forks source link

Update infrastructure to avoid "fat" Candela bundle #503

Closed waxlamp closed 7 years ago

waxlamp commented 7 years ago

UPDATE: further changes were made on this PR superseding the description below the break.

This PR introduces three ways to incorporate Candela into a client project:

  1. Easy mode. Include one of the built bundles - candela-all.js or candela-all.min.js in a <script> tag or a webpack project. These bundles contain everything available in the codebase, bundled together with all dependencies, forming a large but ready-to-go bundle.

  2. Medium mode. Candela now partitions the built-in components into "plugins"; each plugin has an index.js file exporting the components, and a load.js file that automatically adds those components to candela.components. This method requires using Webpack for the client project, and results in reasonable bundle sizes, only including whatever the plugins you load require. Since the other components come more or less for free once the dependencies for a given plugin are included, this caps the maximum size of your resulting application bundle.

  3. Beast mode. You can forgo the plugin loaders by importing or require()ing just the exact files/components you need for your project, again using Webpack. This approach will theoretically give you the smallest bundle size for the portions of Candela you wish to use.

TODO:

(the following is outdated)

Previously, we were building Candela like an application bundle, which in particular meant bundling all dependencies into the resulting code bundle.

The main change here is to use the externals option of Webpack to avoid doing so. Instead, it will now be incumbent on the client to include the correct libraries when using Candela. The final bundle will still be relatively large, but won't include anything that's not being used, and won't include multiple copies of a single library, etc.

This PR also builds multiple plugin "sub-bundles" to go along with candela.js and candela.min.js. Clients can include these directly, or they can reference the source files and use Webpack to build the final application bundle, etc.

TODO:

codecov-io commented 7 years ago

Codecov Report

Merging #503 into master will decrease coverage by 0.71%. The diff coverage is 95.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
- Coverage   48.43%   47.72%   -0.72%     
==========================================
  Files          40       62      +22     
  Lines        1627     1645      +18     
==========================================
- Hits          788      785       -3     
- Misses        839      860      +21
Impacted Files Coverage Δ
plugins/mixin/Events.js 100% <ø> (ø)
plugins/trackerdash/TrackerDash/colors.js 100% <ø> (ø)
plugins/vega/index.js 100% <ø> (ø)
plugins/trackerdash/TrackerDash/utility.js 26.66% <ø> (ø)
plugins/mixin/Resize.js 50% <ø> (ø)
plugins/mixin/InitSize.js 33.33% <ø> (ø)
plugins/trackerdash/TrackerDash/styles/index.js 100% <ø> (ø)
plugins/mixin/AutoResize.js 37.5% <ø> (ø)
plugins/mixin/VegaChart.js 83.87% <ø> (ø)
plugins/trackerdash/TrackerDash/ResultTablePane.js 9.45% <100%> (ø)
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c8a1b2...237f28b. Read the comment docs.

waxlamp commented 7 years ago

@jeffbaumes PTAL.

I realize there are some major changes in this PR, so if you'd like to schedule a time to look this over together, please let me know.

jeffbaumes commented 7 years ago

I get the following on npm run build:examples (happened 2x in a row):

> candela@0.0.0-semantically-released build:examples /home/jeff/code/candela
> cd examples; webpack

<--- Last few GCs --->

   30194 ms: Mark-sweep 1375.0 (1435.4) -> 1370.4 (1435.4) MB, 1839.5 / 0.0 ms [allocation failure] [GC in old space requested].
   32067 ms: Mark-sweep 1370.4 (1435.4) -> 1370.4 (1435.4) MB, 1872.9 / 0.0 ms [allocation failure] [GC in old space requested].
   33931 ms: Mark-sweep 1370.4 (1435.4) -> 1371.9 (1418.4) MB, 1863.2 / 0.0 ms [last resort gc].
   35623 ms: Mark-sweep 1371.9 (1418.4) -> 1373.5 (1418.4) MB, 1692.2 / 0.0 ms [last resort gc].

<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x39e6585cfb39 <JS Object>
    1: new constructor(aka Binding) [/home/jeff/code/candela/node_modules/babel-traverse/lib/scope/binding.js:~12] [pc=0xe85cbd3137c] (this=0x213ebd4b83a1 <a Binding with map 0x2d3f70e41f1>,_ref=0x213ebd4b8361 <an Object with map 0x2d3f70e4039>)
    3: registerBinding [/home/jeff/code/candela/node_modules/babel-traverse/lib/scope/index.js:~544] [pc=0xe85cbd568ac] (this=0x213ebd4b7c21 <a Scope w...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
 1: node::Abort() [node]
 2: 0x10d2fbc [node]
 3: v8::Utils::ReportApiFailure(char const*, char const*) [node]
 4: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [node]
 5: v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [node]
 6: v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object**, v8::internal::Isolate*) [node]
 7: 0xe85cb1079a7
Aborted (core dumped)
waxlamp commented 7 years ago

Hmm, that looks painful. What version of node are you running?

On Thu, Jun 22, 2017 at 10:00 AM Jeff Baumes notifications@github.com wrote:

I get the following on npm run build:examples (happened 2x in a row):

candela@0.0.0-semantically-released build:examples /home/jeff/code/candela cd examples; webpack

<--- Last few GCs --->

30194 ms: Mark-sweep 1375.0 (1435.4) -> 1370.4 (1435.4) MB, 1839.5 / 0.0 ms [allocation failure] [GC in old space requested]. 32067 ms: Mark-sweep 1370.4 (1435.4) -> 1370.4 (1435.4) MB, 1872.9 / 0.0 ms [allocation failure] [GC in old space requested]. 33931 ms: Mark-sweep 1370.4 (1435.4) -> 1371.9 (1418.4) MB, 1863.2 / 0.0 ms [last resort gc]. 35623 ms: Mark-sweep 1371.9 (1418.4) -> 1373.5 (1418.4) MB, 1692.2 / 0.0 ms [last resort gc].

<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x39e6585cfb39 1: new constructor(aka Binding) [/home/jeff/code/candela/node_modules/babel-traverse/lib/scope/binding.js:~12] [pc=0xe85cbd3137c] (this=0x213ebd4b83a1 <a Binding with map 0x2d3f70e41f1>,_ref=0x213ebd4b8361 <an Object with map 0x2d3f70e4039>) 3: registerBinding [/home/jeff/code/candela/node_modules/babel-traverse/lib/scope/index.js:~544] [pc=0xe85cbd568ac] (this=0x213ebd4b7c21 <a Scope w...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory 1: node::Abort() [node] 2: 0x10d2fbc [node] 3: v8::Utils::ReportApiFailure(char const, char const) [node] 4: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [node] 5: v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [node] 6: v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object*, v8::internal::Isolate) [node] 7: 0xe85cb1079a7 Aborted (core dumped)

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/Kitware/candela/pull/503#issuecomment-310389009, or mute the thread https://github.com/notifications/unsubscribe-auth/ACxNJMWqp0GUmwOj6cVhFcZWWZDe9hxgks5sGnORgaJpZM4N5HRL .

jeffbaumes commented 7 years ago

node version 6.10.2

waxlamp commented 7 years ago

Can you try with node 7?

roni

On Thu, Jun 22, 2017 at 12:17 PM Jeff Baumes notifications@github.com wrote:

node version 6.10.2

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/Kitware/candela/pull/503#issuecomment-310429588, or mute the thread https://github.com/notifications/unsubscribe-auth/ACxNJDqgR0vAF42gGl_aWoEdXLvbXjb9ks5sGpOdgaJpZM4N5HRL .

jeffbaumes commented 7 years ago

Similar behavior on node 7.10.0 (npm version 4.2.0).

jeffbaumes commented 7 years ago

I was able to get the candela-all bundle working in a script tag, but there is strangeness in calling it candela-all. Since it is not a valid JS name, I could only get it by using an accessor off of the window object. Perhaps a reason to just call the bundle candela?

This template works if you place it as index.html in the root of the Candela repo and serve the directory.

<body>
<script src="dist/candela-all.js" charset="utf-8"></script>
<script>
  var candela = window['candela-all'];
  console.log(candela);
  var el = document.createElement('div')
  document.body.appendChild(el);

  var data = [];
  for (var d = 0; d < 10; d += 1) {
    data.push({
      a: d,
      b: d
    });
  }

  var vis = new candela.components.BarChart(el, {
    data: data,
    x: 'a',
    y: 'b'
  });
  vis.render();
</script>
</body>
waxlamp commented 7 years ago

I think calling it candela.js is fine, I'll make the appropriate change.

It is really cool that we can just drop an HTML file in the repo root, build the project, and have a working "app" just by serving that file. Do you think we should include an example like that in the repo? Maybe as a one-off in the examples directory?

jeffbaumes commented 7 years ago

Sure I like that idea - would be great if it worked with the run:examples npm task but I don't think that one currently serves high enough in the source tree to be able to refer to the dist directory, am I correct? Having a second run:serve or something similar might be convenient (or change the examples one to serve the root).

waxlamp commented 7 years ago

@jeffbaumes, take a look at 237f28b (changes candela-all to candela in the dist directory).

I'll open an issue about the HTML example - I think I'd rather just get this PR merged at this point.

waxlamp commented 7 years ago

@jeffbaumes thanks for the (unusually painful) review!