Minim 1.0 Proposal about minim

[smizell]

cc @danielgtaylor

[danielgtaylor]

👍 Great stuff!

My only comment is regarding ES6/ES5 classes and how to handle them. I am strongly in favor of using ES6 classes, defaulting to ES6 in examples, and generally recommending people use ES6 inheritance if possible. We should also document how to use ES5 if it isn't possible.

The _.extend syntax is interesting but note that you can also do it by hand, and you can simulate super calls easily. I'm definitely open to sugar wherever possible, but here's a quick sugarless example showing how it can work:

// This is our base class, defined in ES6
class MyObject {
  constructor(id) {
    this.id = id;
  }

  hello() {
    return `Hello, id ${this.id}!`;
  }

  bye() {
    return 'Goodbye!';
  }
}

// Now, let's pretend we have to use ES5
function MySubClass(id, name) {
  // Simulate a super call
  MyObject.bind(this)(id);
  this.name = name;
}

// Make it a subclass by copying the prototype
MySubClass.prototype = Object.create(MyObject.prototype);

// Now we can overwrite any methods we like
MySubClass.prototype.hello = function hello() {
  return `Hello, ${this.name}! (id:${this.id})`;
};

const test = new MySubClass(1, 'world');
console.log(test.hello());
// => Hello, world! (id:1)
console.log(test.bye());
// => Goodbye!

Hopefully this shows that we really don't need to move away from ES6 classes. You can run the above via babel-node classtest.es6.

[smizell]

Good stuff, and great example. I want to say I'm very torn on this. I totally prefer the ES6 syntax for classes, but here are my current issues. I believe there are issues for users and big issues for developers.

First, the issues for users of the library. I'm assuming that a majority of people will not being using ES6. We won't want to require people to subclass like you've shown above, so we'll have to provide something like StringElement.extend. So now we have to support this forever, basically, unless we want to stop supporting ES5 at some point.

Whether we provide this in an ES6 class system or have it in ES5, we are not preventing the user from using it with ES6 classes. Take for instance this article where the user is using ES6 classes with Backbone "classes." It is still usable and is transparent.

Now if we stopped here with the issues, I would see no big difference in writing Minim in ES6 and adding this special extend method for classes and would 100% vote for that. But the problem with working with this trickles down to the developer in a big way. I have actually been quite frustrated with using ES6 for this library.

• Running tests takes at least five seconds each time. One second is devoted to linting, and the rest to transpiling. This is unbearable to me. I should be able to run my tests with nodemon or something and they finish by the time I glance at my terminal.
• No source maps, so errors in failing tests provide little help. The code Babel produces is unreadable to me.
• Related: I am fearful of supporting this library. If a user opens an issue saying they got an error on such and such line number, I will have to dig through two codebases... the transpiled and the ES6. If I don't now why the error is happening, it may be because of Babel magic.
• Code coverage tools provide incorrect data since they run against transpiled code. It's not even useful to look at it! Those tools are for helping you know what you haven't tested, and that data isn't there.
• I wanted to try out Flow, and it doesn't work for the new for loops in ES6 along with files ending in .es6. I switched to .js, but I get errors when I ran it because of the loops. This made Flow not really helpful.

With these things in mind, the development process for Minim is quite bad, especially for such a small library. The only win I see us getting here is nicer syntax. There is no reason users of the library couldn't use it with ES6, so we are OK there. And if we write this using old ES5 "classes," we can still use it on future platforms that support ES6, even with ES6 classes. Lastly, we'll always have to support this special .extend functionality, so there is no getting away from it.

---

Hopefully this doesn't seem like a rant (maybe a partial one). I am just struggling to seeing the benefit of using ES6 for a library like this with all of this pain I'm having. For the browser, it makes total sense because you will always have a build step. With this, the build step is unecessary. What are your thoughts on this? @danielgtaylor

[smizell]

Writing this with prototypical inheritance is also still usable with CoffeeScript classes.

MyClass = (@id, @name) ->

MyClass::hello =  -> return "Hello, #{@name} with id #{@id}"

class MySubClass extends MyClass

test = new MySubClass 1, 'John'
console.log test.hello() # Hello, John with id 1

Also, wanted to make a quick comment on this:

You can run the above via babel-node classtest.es6

This actually will transpile then run your code. Babel-node takes 0.6 seconds to run the code below :)

console.log('Hello world');

Node 0.12 takes 0.043 seconds.

[danielgtaylor]

Love the rant - get it all out! 👊

First, are you suggesting we rewrite in vanilla ES5? It seems like many of these issues are going to be problem with any transpiled language, including CoffeeScript (though to a lesser degree).

we'll have to provide something like StringElement.extend. So now we have to support this forever, basically, unless we want to stop supporting ES5 at some point.

It could maybe be an extra module that you load to provide compatibility. If and when ES6 becomes the norm we could phase it out without a major version bump in Minim:

var minim = require('minim');
minim.use(require('minim-extend'));

Yes, in the short term it's still something we will maintain, and we would need to think about how best to test it to make sure we never break compatibility, but it gives us a path to not maintain it forever. Just a thought.

Running tests takes at least five seconds each time. One second is devoted to linting, and the rest to transpiling. This is unbearable to me. I should be able to run my tests with nodemon or something and they finish by the time I glance at my terminal.

This has really frustrated me too. What's happening is that we are spawning node, loading npm, spawning node again, loading eslint, running it, spawning node again, loading babel, etc etc. The biggest offender by far though is babel itself. I just hacked up a quick Gulpfile to try it there and that shaves off about a couple seconds, but it's still fairly slow. One thing that would make it much faster is adding a watch command (saving startup time of a couple seconds). Here's me file if you want to play with it:

var gulp = require('gulp');
var eslint = require('gulp-eslint');
var babel = require('gulp-babel');
var mocha = require('gulp-mocha');

gulp.task('compile', function () {
  return gulp.src(['src/**/*.es6'])
    .pipe(eslint())
    .pipe(eslint.format())
    .pipe(eslint.failOnError())
    .pipe(babel())
    .pipe(gulp.dest('lib'));
});

gulp.task('default', ['compile'], function () {
  require('babel/register');
  gulp.src(['test/**/*.es6'])
    .pipe(mocha());
});

No source maps, so errors in failing tests provide little help. The code Babel produces is unreadable to me.

Both Babel and gulp-babel provide a source map option. We should try it out, but I don't have much experience with source maps honestly.

Related: I am fearful of supporting this library. If a user opens an issue saying they got an error on such and such line number, I will have to dig through two codebases... the transpiled and the ES6. If I don't now why the error is happening, it may be because of Babel magic.

Is the location fixed by generating source maps? One option we have here is to remove any features that require the babel-runtime (like Symbol) and then each file will have a bit of code dumped in the top to handle whatever features that file is using. It's similar to how CoffeeScript handles it, but I'll warn you it is a bunch of difficult to read code. If we don't use the runtime then all the code run by our downstream is in our lib folder, which may have some advantages. Still not perfect though, and it'll duplicate a lot of code.

Code coverage tools provide incorrect data since they run against transpiled code. It's not even useful to look at it! Those tools are for helping you know what you haven't tested, and that data isn't there.

Do source maps fix this one too? I know it was possible with CoffeeScript with some magic - maybe we can figure out something for this.

I wanted to try out Flow, and it doesn't work for the new for loops in ES6 along with files ending in .es6. I switched to .js, but I get errors when I ran it because of the loops. This made Flow not really helpful.

Does Flow not have these same issues if you annotate with types? Or do you plan to keep vanilla JS and just run Flow on that?

You can run the above via babel-node classtest.es6

Yeah it's just a quick way to run some ES6 without having to compile, then run as two steps. It's slow but convenient when testing stuff out. 😄

---

Anyway, you make some good points and I'm not totally against using ES5. I was hoping that we could make ES6 work and use this project as a small example for future projects. I agree that some of these issues are a pain, but I hope that maybe we can figure out a way to make it work. We can't be the first people running into this!

[smizell]

Woah, awesome response. It's late here but wanted to note a couple of things.

First, I'm suggesting we consider ES5 for this library. Not necessarily saying we should at this point, but it solves all of the problems I outlined.

Second, I think ES6 is catching on in the browser because you always have a build step and need source maps. Server side code seems like a different problem area, which makes me wonder if people are using ES6 this way.

Last, I had planned on just running flow on top of the code to get make sure there weren't any compile time type issues. Would want to add another build step back in.

[smizell]

Closing, as most of these are addressed or captured in other issues.