Skip to content
This repository has been archived by the owner on Sep 7, 2022. It is now read-only.

✨ Allow ES6 classes #7

Merged
merged 1 commit into from
Apr 12, 2017
Merged

✨ Allow ES6 classes #7

merged 1 commit into from
Apr 12, 2017

Conversation

marvinroger
Copy link
Contributor

@marvinroger marvinroger commented Apr 11, 2017

This stays ES5 compatible and allow usage of ES6 classes. See this link for the explanation.

Basically, this allow this smoother syntax:

const util = require('util')
const Scout = require('zetta-scout')

// "Old" syntax

const TestScout = function () {
  Scout.call(this)
}
util.inherits(TestScout, Scout)

TestScout.prototype.init = function (next) {
  next()
}

// "New" syntax

class TestScoutEs2015 extends Scout {
  init (next) {
    next()
  }
}

Both syntaxes still work with this PR.

@@ -35,8 +35,7 @@ exports.create = function(/* constructor, ...constructorArgs */) {
var machine;

if (constructor.prototype) {
machine = Object.create(constructor.prototype);
machine.constructor.apply(machine, constructorArgs);
machine = new (Function.prototype.bind.apply(constructor, [null].concat(constructorArgs)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marvinroger Should [null] here be [constructor.prototype]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because the first element given to bind (the context) is overwritten by the new. See http://stackoverflow.com/a/35878917

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A-okay. Got it. 👌

@kevinswiber
Copy link
Member

@AdamMagaluk This looks good to me. Any objection to merging? (Asking permission, because well... you know... it's complicated. 😉)

@AdamMagaluk
Copy link
Contributor

LGTM. Sorry for the delay @kevinswiber and @marvinroger been busy with other projects this week. I hope to give all the other PRs a review in the next few days.

@kevinswiber
Copy link
Member

@AdamMagaluk Your "delay" was 12 hours. Go easy on yourself. I've committed far more atrocious open source crimes. 😺

@marvinroger
Copy link
Contributor Author

Don't worry, moreover we don't live in the same TZ (Europe/Paris for me) so there will be delay anyway. 😉

@AdamMagaluk AdamMagaluk merged commit ab4bc60 into zettajs:master Apr 12, 2017
@marvinroger
Copy link
Contributor Author

Thanks! Can you make a release?

@AdamMagaluk
Copy link
Contributor

Doing it now.

@AdamMagaluk
Copy link
Contributor

v0.8.0 Released.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants