hammerlab / cycledash

Variant Caller Analysis Dashboard and Data Management System
Other
36 stars 2 forks source link

Use the newest version of pileup with require support #726

Closed armish closed 9 years ago

armish commented 9 years ago

Review on Reviewable

ihodes commented 9 years ago

Nice!

On Fri, Jun 12, 2015 at 7:02 PM, B. Arman Aksoy notifications@github.com wrote:

armish commented 9 years ago

It took me a while to fix the React clashing problem, but turns out it was just uglifyify's old implementation that was behaving badly :\

Have a nice weekend and safe travels, @ihodes!

danvk commented 9 years ago

@armish is this good to merge? Or do you want to find a different (flow compatible) way to resolve the browserify issues?

armish commented 9 years ago

@danvk haven't resolved the flow issue yet. Other than the flow issue, this works fine; but, feel free to hold off from merging this before we completely resolve issues on pileup.js side.

danvk commented 9 years ago

Let's go ahead and merge this—it should be the same after we fix things up on the pileup side.

Could you check what the size of cycledash/static/js/dist/bundled.js is before & after this change? I'm hoping that this results in Cycledash and pileup.js sharing copies of React, D3 and underscore.

armish commented 9 years ago

unfortunately this changeset does not make them share those libraries yet, as pileup.js is still included as a bundled file with all the libraries in it. The size of the bundled.js was 620K before the changes and it is now ~1M. The new uglifyify seems to be doing a worse job :\