Use es6 modules and webpack in prep for NPM publish

[jaimedp]

A lot of changes. I tried to do the messy change of removing and re-indenting files on the first commit and then fixes on the following commits to make it a bit easier to see, but it is still hard to see the changes :(

[narenranjit]

Great! Adding ?w=1 to github diff url ignores whitespace so wasn't too bad to see changes :) Few thoughts:

• Maybe I missed it but are we not exposing nwt publicly? Some examples like (http://forio.com/contour/gallery.html#/chart/scatter/scatter-trendline) depend on a few functions from that so we should.
• We should probably export it as Contour.utils instead?
• Can we replace the lodash functions in contour-utils with the actual lodash imports? We can import those specific functions without bundling all of lodash.

[narenranjit]

Also, in the spirit on modernizing jshintrc -> eslintrc ?

[narenranjit]

Any reason not to export function addAxis instead? afaik exporting a const arrow function will show up as anonymous in stacktraces, while exporting named functions will list it out.

[jaimedp]

I don't think it makes any difference for the stacktrace, I test all the options and the traces still get the correct function name... then found this
http://lukecod.es/2015/12/01/es6-arrow-function-stack-traces/

[narenranjit]

What're the before/after file-sizes of contour.min.js?

[jaimedp]

• I added Contour.utils to expose nwt to the rest of the world.
• I think we can keep the custom lodash replacements for now.. I think breaking up with lodash should be complete :)
• I have another branch with eslint and all the fixes for linting but I think we should finish and merge this branch first
• The size of the new min.js file is ~82k, the previous one was ~78k, so is ~4K larger for the stuff that webpack injects I guess

Also, for publishing to npm, you'll need to create an npm user for forio and make sure the author info in package.json matches with the user, so we can publish it.

[narenranjit]

Looks good!

• Lodash is overkill for the isObject etc of course, but why not use it for the merge, defaults etc? The less code under our maintenance purview and the more we handoff to established libraries the better, unless there's a reason not to?

• We should whitelist IE11 in preset-env.

[jaimedp]

• Lodash... Yeah, it makes sense.
• IE11... I was thinking the same thing last night, I guess I forgot to do it :)

I'll update the PR tonight.

One more thing to note. I removed the /dist folder from the repo, this means that you will no longer be able to use bower install since it does not have a post-install build. Just to make sure you're ok completely dropping support for bower.

Also, we need to remember to update contour.geo too with this changes.

[narenranjit]

I'd forgotten bower existed till I tried to build contour last week, so fine dropping it :)

Tangentially, we've given up on making millions with Contour-advanced so contents of that repo should go somewhere public, just not sure where. Thoughts? Just new showcase examples? (I'll get you access to the advanced repo in a bit).

[jaimedp]

Moved to use lodash.merge and lodash.defaults (increase in size of the minified file by ~13k). Also added ie11 to the env in babel and fixed 267 & 268.

Regarding advance viz, I think it would be good if they had their place in te showcase and then you could just install them like npm install contour.drawable or something like that

[narenranjit]

I'm having second thoughts about not including dist, though more about workflow than anything technical.

For new PRs it's useful to have devs include a jsfiddle in their example pointing to a jsfiddle demoing their change, for e.g. see #251 which has http://jsfiddle.net/3yqyvdud/4/ which points to a pre-built contour file for that PR, pulled from the incoming dist. This really reduces the friction of testing PRs, and puts the onus of making sure it builds/works on the submitter.

Thoughts/ suggestions for a better workflow?

[jaimedp]

Hey, I see the point of evaluating PRs from a jsfiddle, but I think you'd end up with unnecessary merge conflicts which is minor, but the bigger problem I think it is that you end up with a very confusing versioning, since the /dist folder no longer contains the "official" released version, it has whatever the latest merge puts there, if it didn't forget to build it... for example in cases where the PR does not need to be js tested (ie. changes to less files or docs), then the /dist folder is not even the latest/edge version, is just a random snapshot.

There is one posible solution, but it would mean to drop the bundle of the lodash merge/default functions. The solution is to include the /src/scripts/index.js file as a <script type="module"> that way (with some tweaks) it works, but it limits to import only files addressable from the src folder (ie. no node_modules).

I did a proof of concept jsfiddle here

The changes required for this to work an on this commit

I still don't think it would be a good solution, since it imposes an unnatural limit to what you can use, but I wanted to test what it would look like :)

[narenranjit]

You can prevent merge conflicts / large diffs on dist if you mark minified files as "binary" in gitattributes (see https://github.com/forio/flow.js/blob/master/.gitattributes for e.g. https://blog.andrewray.me/dealing-with-compiled-files-in-git/ for more info).

I don't think dist contour.js being of uncertain freshness is that big a deal, if you're explicitly copying source from github instead of cdn/npm you hopefully know what you're doing :)

Source type module is an interesting idea, let me play around with that.