Vexflow: Modern Module System + ES6 (via Babel)

[Silverwolf90]

The current module system + manual management of dependencies is really out-dated, it would be ideal to move to a modern module system, either ES6 Modules or CommonJS.

I think ES6 modules are preferable, but would come with the additional burden of a transpilation step (usually with Babel). However, VexFlow is already a compiled target so this wouldn't significantly change the current workflow.

In general, how do we feel about using ES6? @0xfe what browsers do you intend for VexFlow to support? The readme says "modern browsers" but that's rather vague. Barring modules, most of the spec is supported by modern browsers and wouldn't require a transpilation step, but would come at the cost of supporting outdated browsers.

It would also nice to have a build step that has a watcher with a cache, right now grunt watch takes about 4.3s to recompile. That's a feedback loop that could be sped up significantly.

[mscuthbert]

I'm thinking about this a lot with music21j -- my sense is that ES6 modules are not yet ready for prime time. There needs to be at least one major browser for each OS that supports a feature out of the box (for testing w/o Babel transpilation) before I'm willing to go with it. (Edge preview includes it -- so at least we're on the way). I'd say to wait a bit on making a change since CommonJS isn't where I'd want to be in two years, and ES6 Modules aren't where we can be now.

Moving to ES6 classes and objects, OTOH, is something I'd endorse doing for the next VF version (version numbers and releases are important) and setting that as a browser minimum.

[Silverwolf90]

I think a major version bump would be very beneficial for VexFlow. It would be a good place to batch a lot of work -- especially SMuFL support -- which would probably benefit from breaking the current API a bit.

[0xfe]

@Silverwolf90, Those were 2010 goals, and lots has changed since then. (Canvas works everywhere today!)

Things I would love do today:

• All ES6 (transpilation is fine)
• Incremental build system
• SMuFL
• Consistent style across code base
• Probably just SVG (no need for Canvas)

(Support for popular HTML5 compliant browsers (Chrome, Firefox, etc.), especially on mobile.)

[0xfe]

Re: version bump, I think it's fine to do for API reasons, but not for batching features.

I prefer features to be added incrementally and tested by the outside world as early and often as possible. This approach has slightly higher overhead, but typically (IME) leads to more predictable and higher quality releases.

So, the approach I would take is create a new 1.0 branch and leave it as is. Start new feature development on master (keeping tests always green) -- this can be 2.0. We can always patch in bugfixes or other minor things into the 1.0 branch if necessary.

[Silverwolf90]

• All ES6 (transpilation is fine)
• Consistent style across code base

I think going all ES6 is the way to go as well. @mscuthbert I agree that ES modules aren't quite ready for prime time, but the syntax is very unlikely to change and the transpiled versions work despite the differing loading semantics. Personally, I have not felt much pain when using them.

What about toolchain, should we stay with grunt? For bundling, use browserify or webpack? How would you you want the codebase converted? There are tools like lebab which could make conversion easier.

I'm thinking grunt-babel (using babel with babel-preset-es2015) + grunt-browserify is probably the simplest way to go since we're already using grunt.

re: major version

Completely agree! I know there has been hesitancy to break backwards compatibility in the past, but I guess that was before we were using semver.

[0xfe]

I like webpack, but I think using it alone will be messy (especially if you want multiple environments.) Perhaps grunt + webpack + babel. For the interim, grunt-babel sounds like the right approach.

I've never used lebab before, but if it works well and the output doesn't look machine-generated, then why not. We'd probably need a combination of lebab + hand tuning, but I'm open to experimenting.

[aaronmars]

This is an area where I have quite a bit of experience. I've spent the last 1.5 years converting a huge, global var-based library to ES6 modules. That project, of course, went through an evaluation phase where CommonJS was considered. The thinking was that CJS will _always_ require a build step as it will never run natively. So, if we're going to incur this build-time processing, let's do it in a way that that has native support on the horizon, and then, can go away (as soon as IE11 support is gone and the modern browsers are auto-updated with modules support).

VexFlow is a perfect candidate for such a conversion:

1) The architecture is quite sound. If you were to UML this thing out, it would look great WRT the modularity and inheritance.

2) There are no dependencies (now that Raphael is gone). In my experience, this was the most complicated part of getting a module system going.

As for specific tools, it's probably best to define what the target environment would be. If we're OK with shipping ES6 syntax that only has the module syntax transpiled, I'd look into Rollup (http://rollupjs.org/). If we want to target ES5 platforms, then Babel can be integrated. I just don't see a call for webpack or browserify here...

Other awesome tools in this space are jspm and SystemJS, but for a pure library project like this, they're not necessary, I think.

_I want to start on this ASAP!_ It's huge, and will open up so many doors with the flexibility that a modular system will provide.

[Silverwolf90]

@aaronmars Really good points! I completely agree.

I've heard good things about rollup but have not used it personally. It's definitely worth looking into, as I do feel that webpack might be too heavy-handed for VexFlow's needs. I haven't used browserify in a really long time, but it's just generally the other option mentioned alongside webpack.

[0xfe]

Thanks @aaronmars. Rollup looks pretty good -- never heard of it before.

Re: transpilation, I would really like to keep it transpiled until we are confident that there is stable support across the major desktop and mobile browsers.

[aaronmars]

For sure. Those are tools geared toward CommonJS sources, right? I don't actually know of a use for them with ES6 modules... but I have stale Browserify experience and zero webpack experience.

I'd be down to try an initial conversion to modules that uses Rollup to generate a functionally-equivalent binary. The hard part is restraining myself and not putting in a bunch of class definitions, getters/setters, etc. along with it! To @0xfe 's point, it would probably be best to keep these huge changes incremental.

Currently, vexflow is 100% ES5 compliant. Bringing in language features that aren't supported in IE11* is a separate step, and something Rollup doesn't handle (that's a job for Babel). It would be a good exercise to only introduce the module syntax, have that transpiled by Rollup, and leave the rest of the code as untouched as possible. Basically, we'd be introducing module syntax and transpiling that out of necessity (through Rollup) since no browser knows what to do with it yet.

• Side note: When you think about it, IE11 is the real blocker here. I'm assuming IE 8 and 9 are not considered "modern", and everything else has a pretty good auto-update mechanism that is going to change how JS is developed, IMO.

[mscuthbert]

Great! I was the only one who expressed any reservation about moving to ES6 modules, but it sounds like there is so much excitement that I will officially drop that reservation (and I'm excited to learn to use them too).

The only issue w/ dropping support for IE11 is that many related MIDI applications can run on IE 11 but not on Edge (because IE11 supports NPAPI plugins which include the Jazz Plugin for MIDI and others, while Edge neither supports NPAPI nor the Web MIDI API)

[0xfe]

@mscuthbert We're not suggesting dropping support IE11. We'll need to use Babel and transpile ES6 code until we have good ES6 support and stability across browsers.

@aaronmars Yes, IE 8 and 9 are out of scope.

[jmahmud]

Hi all!

I'm not really going to add anything of use here - but would like to share my enthusiasm for introducing a modular system into Vexflow and I too concur with @aaronmars suggestion that the system adopted should be the most standards compliant / allows native support in the future. Is is worth beginning a new branch?

[aaronmars]

Hey, I've got these things working:

• grunt build with Rollup
• Quint tests running and passing with no modification to the test code
  
  
  
  • This is true for the grunt environment as well as the visual browser environment
  
  

I still need to tweak the node functionality. The good news is that's necessary in order to get the visual regression tests running. I don't see why that will take too long.

I can share my working branch if anyone's interested!

[aaronmars]

Well, that was a bit easier than I thought...
Here's the branch. I'll send a PR if that would be easier, but I thought you all might want to look at it before I send it along.

https://github.com/aaronmars/vexflow/tree/es6_modules

[Silverwolf90]

@aaronmars That's really awesome! Definitely open up a PR.

[aaronmars]

OK. PR coming... Perhaps I could get a few questions answered first?

Other than the module-related changes, here are a few things I had to change:

• Removed a circular dependency between Music and KeyManager. Moved KeyManager.scales to Music.scaleTypes
• Combined flow.js into tables.js. It's a bit weird to do import { Flow } from 'tables';. Should I delete flow.js or delete tables.js?
• Removed the flipRectangle() function from SVGContext. This was doing 2 lines of gymnastics using the arguments object with side effects. It was causing issues in my local build, and I figured the repeated code is better than the over-engineered solution.
• The Flow.renderGlyph() function is defined in glyph.js. This is weird, but I left it in to keep the refactors minimal.

One more question: Should index.js be in src/ or just in the root? It's sort of the spec for the structure of the Vex.* global. I have no opinion either way...

[0xfe]

On Thu, Jun 16, 2016 at 4:01 PM, Aaron [email protected] wrote:

OK. PR coming... Perhaps I could get a few questions answered first?

Other than the module-related changes, here are a few things I had to
change:

• Removed a circular dependency between Music and KeyManager. Moved
  KeyManager.scales to Music.scaleTypes
• Combined flow.js into tables.js. It's a bit weird to do import {
  Flow } from 'tables';. Should I delete flow.js or delete tables.js?

How about deleting flow.js and moving Flow to vex.js?

• Removed the flipRectangle() function from SVGContext. This was doing
  2 lines of gymnastics using the arguments object with side effects. It was
  causing issues in my local build, and I figured the repeated code is better
  than the over-engineered solution.
• The Flow.renderGlyph() function is defined in glyph.js. This is
  weird, but I left it in to keep the refactors minimal.

Maybe track this weird stuff in a bug, so we can clean it up later.

One more question: Should index.js be in src/ or just in the root? It's
sort of the spec for the structure of the Vex.* global. I have no opinion
either way...

Let's put it in src/ for now. If it becomes a problem, we can change it.

Overall, everything looks really good. Have you run the regression tests?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/0xfe/vexflow/issues/352#issuecomment-226596856, or mute
the thread
https://github.com/notifications/unsubscribe/AAOuk6OEc8nwSlnsy0QiJ95Enq3siaFSks5qMauWgaJpZM4Iz15B
.

Mohit Muthanna [mohit (at) muthanna (uhuh) com]

[aaronmars]

Yup! Tests look 100% to me. The following super command™ is what I'm basing that on:
grunt;grunt test;npm run generate;npm run diff;cat build/images/diff/warnings.txt

...in addition to loading tests/flow.html manually and verifying everything passes with no console errors.

It's probably best to have the file that defines Vex things in vex.js, and have the file that defines Flow things in its own file. For now, I'll leave tables.js, and delete flow.js.

[aaronmars]

Here it is: https://github.com/0xfe/vexflow/pull/357

EDIT: Just kidding... Here it is! https://github.com/0xfe/vexflow/pull/358
(sorry for the confusion)

[Silverwolf90]

Closing since #358 was merged.