search

concord-consortium / lab

HTML5-based scientific models, visualizations, graphing, and probeware
http://lab-framework.concord.org/
MIT License
57 stars 38 forks source link

Refactoring and build system cleanup #148

Closed pjanik closed 4 years ago

pjanik commented 4 years ago

It's probably impossible to review this PR in a traditional way. I hope that list of commits might help and I'll try to summarize all the changes here.

  1. Node v12 is supported.
  2. CoffeeScript has been removed.
  3. Modules syntax has been changed from AMD/RequireJS to modern ES6 syntax.
  4. JavaScript build system has been changed from RequireJS to Webpack and Babel.
  5. Some new bugs showed up because of that, but they have been fixed.
  6. All the tests have been converted to Jest (from Mocha and Vows).
  7. HAML files are gone, there are HTML files with little templating handled by HTMLWebpackPlugin.
  8. SASS is also compiled by Webpack and appropriate loaders.
  9. Multiple Ruby scripts have been converted to JS/Node scripts, so the build system doesn't require Ruby and any ruby gems. There are some Ruby scripts left, but these are tools that are most likely not useful at this point. Or at least not useful for regular development.
  10. Most of the Git Submodules have been removed and replaced by NPM packages. There are a few left, but it's mostly our own libraries.
  11. Makefile size has been greatly reduced (87 lines). It's mostly copying resource files from src to public. It could be easily replaced by NPM scripts, but I'm running out of time. It's very unlikely developer would need to touch it.
  12. Lab hard dependencies are limited to jQuery and jQuery UI. Other libraries are bundled in lab.js and lab.css files.
  13. Multiple unnecessary files have been removed and many others cleaned up.

I went ahead and deployed this branch as version 1.17.0-pre.1. I also setup Lab Interactives Site to use this tag as staging, so this build can be demoed and it's ready for QA:

http://lab.concord.org/embeddable-staging.html#interactives/samples/1-oil-and-water-shake.json

If you open JS console, you'll see multiple errors saying that some JS and CSS files haven't been found. That's fine, as these libraries are now bundled in lab.js and lab.css. Once we update production, I'll update Lab Interactive Site(s) not to require these files anymore.

I had to update interactives.html/application.js to fix one error: https://github.com/concord-consortium/lab-interactives-site/commit/83e6054da91bdd4475c0cc9c4ae3f4c43b582c25 But I can still see it: http://lab.concord.org/interactives-staging.html#interactives/samples/1-oil-and-water-shake.json I guess it might be a caching issue. Once it gets updated, this page should work fine too.

stepheneb commented 4 years ago

Very nice! Initial build, run and test process worked on my up-to-date macOS dev machine. Much easier to do development now.

Have wanted to use lab in a current project and thought I'd need to do a bunch of this myself ... which caused me to delay that part ;-)

I saw a related branch: /171206851-maintenance-d3 updating to d3-v5 that has code not in this branch. Are you planning to continue that work?

Coincidently I've done some work in a branch updating lab-grapher for d3 v5 in the last couple of days. Got stuck in D4 because the built-in D3-zoom system for panning and zooming has been re-architected and I think will need to be worked around. See: concord-consortium/lab-grapher#21

pjanik commented 4 years ago

Good to hear that, thanks. :)

I saw a related branch: /171206851-maintenance-d3 updating to d3-v5 that has code not in this branch. Are you planning to continue that work?

Unfortunately, I think we're out of time for more refactoring work. That was all I could squeeze in one week. Frankly speaking, even if there was more time, I think I wouldn't do it after this initial test. I started this branch as I had some problems with the older NPM package. But the most recent 3.x version is modern enough and it works fine both in the web browser and node. So, that's good enough for me (I'm not a big fan of this library :wink:)

The list of breaking changes in v4 is crazy: https://iros.github.io/d3-v4-whats-new/#1 and I'd say some of them are questionable. It doesn't seem like they cared about backward compatibility with v3 or a nice upgrade process. E.g. .attr({prop: value}) doesn't work and only .attr("prop", value) is allowed now. It breaks backward compatibility in a pretty silly way and there are multiple changes like that. Maybe there's some logic behind it but it seems pretty opinionated. I think the worst might be changes to transitions that would probably break DNA animations.