Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: TypeScript / es6 / es7 #6

Closed
ritch opened this issue Jan 10, 2017 · 14 comments
Closed

RFC: TypeScript / es6 / es7 #6

ritch opened this issue Jan 10, 2017 · 14 comments
Assignees

Comments

@ritch
Copy link
Contributor

ritch commented Jan 10, 2017

I think the main goal of loopback-next should be TypeScript and es6-7 as first class features. Below are the goals:

  1. rewrite LoopBack source code as TypeScript classes using es6 / 7 features best practices
  2. use TypeScript native modules eg. import and export.
  3. promises first and everywhere
  4. async and await everywhere

Below is brief list of impact for discussion.

  1. drop callback API ??? - we need to focus on promise support, can we do that without dropping callbacks?
  2. remove all uses of require()
  3. change model script syntax - now export es6 class ???
  4. shouldn't force users to use TypeScript - need to provide docs as both TypeScript and JavaScript
  5. would tests need to be rewritten in TypeScript ???
  6. drop custom impl of mixin and class in juggler

Anything else?

@bajtos @raymondfeng @superkhau

@ritch ritch self-assigned this Jan 10, 2017
@bajtos
Copy link
Member

bajtos commented Jan 11, 2017

I think the main goal of loopback-next should be TypeScript and es6-7 as first class features.

@ritch could you please add arguments supporting this statement? What are the benefits we would like to get from using TypeScript in our code base? What are the costs we have to pay?

While I personally like TypeScript and I think we should make it easy to write loopback applications in TypeScript, at the same time I am not convinced that the loopback core has to be written in TypeScript too.

Here are my concerns:

(1) In Node.js ecosystem, the convention is to use vanilla javascript with no transpilation. This has two aspects:

  • People may be reluctant to use a framework/module that's written in a transpiled language like TypeScript, i.e. we may loose a (big?) part of our user base.
  • We will make it more difficult for people to contribute changes, because people familiar with ES6 only will have to learn TypeScript.

Anyone remember the time 2-3 years ago, when CoffeeScript was the most popular transpiled language? Many people (including myself) were looking down on packages written in CoffeeScript, and contributing patches was definitely a pain.

Do we want LoopBack to become that kind of project?

I know that Angular2 is using TypeScript everywhere. Their situation is different though, because virtually everybody is transpiling their browser scripts these days. Node.js land is different AFAIK.

(2) There are other competing type checkers, most notably https://flowtype.org/. To my knowledge, there is no clear winner in this space yet. Do we want to put our bets on a solution that may become obsolete in the near future?

(3) Performance and usability penalties.

  • async/await is not available natively in any Node.js version yet, 7.x provides async/await behind a feature flag. Transpilers have to either convert the code to vanilla ES5, or use generators introduced by ES6 - I don't know which approach TypeScript is taking. I am pretty sure that generators were not optimized much in V8 yet. My point is that async/await is almost certainly introducing non-negligible performance penalty. Are we ok with that?
  • The performance of built-in Promises is not on par with callbacks yet AFAIK. (Bluebird claims it's at least as fast as callbacks, can we use it with TypeScript?)
  • Promises (at least the built-in V8 Promise) don't play well with post-mortem debugging in Node.js (e.g. node --abort-on-uncaught-exception will report incorrect stack trace in the core dump file produced).

(4) ES6 modules in Node.js are not stable yet. Things may change and we may be forced to update our codebase to accommodate for that. Is the new import/export syntax worth this cost?

In fact, it looks the module loader specification is not complete yet, see https://whatwg.github.io/loader/ (via nodejs/help#53):

Status

This document is a work in progress and dreams of becoming a living standard.

Further reading:


I would prefer to start with a discussion of what benefits we would like to get from new language features available in post-ES5 world (focus on "why" we want to make the change, now "how"/"what").

@bajtos
Copy link
Member

bajtos commented Jan 11, 2017

rewrite LoopBack source code as TypeScript classes using es6 / 7 features best practices

IMO, there are two steps available here:

  1. Start using ES6 class keyword
  2. Start using TypeScript and add additional type information to our classes

If our primary motivation is to start using the class keyword (which is just a syntactic sugar, BTW), then I think we don't need TypeScript for that.

I personally find the class keyword/concept useful and use it in new code. However, I think the benefits of class are not worth the cost of rewriting all our code to use class.

use TypeScript native modules eg. import and export.

See my point above. I think import/export has very little benefits compared to current require(), and considering that the spec is not final yet, I'd prefer to avoid using it.

promises first and everywhere

+1, even though it may cause more difficult post-mortem debugging

TBH, I think this is a change that can be implemented incrementally in the current code base, no need for a big rewrite.

drop callback API ??? - we need to focus on promise support, can we do that without dropping callbacks?

Promise-based APIs can be relatively easily amended to support callbacks, see e.g. Bluebird's .asCallback() API

I am personally fine to move on and drop callbacks entirely.

async and await everywhere

See my comment above, I think async/await is not ready for prime time/production yet.

remove all uses of require()

See my comment above, I think this does not have any significant benefits and is not worth the costs.

change model script syntax - now export es6 class ???

Yes please, this would be awesome! I also think it may not be possible implementation-wise if we want juggler to auto-generate stuff using JSON config, further research is needed IMO.

In fact, I think this is the most important benefit we can offer LoopBack users in loopback-next - the ability to write their models/controllers/etc. using the class keyword in ES6 or using fully-typed TypeScript classes.

However, I am not convinced that switching to TypeScript is necessary in order to get there.

@raymondfeng
Copy link
Contributor

We should also separate the following concerns:

  • Rewrite LoopBack code base with TypeScript
  • Allow application/extension logic to be written in TypeScript in addition to Plain JS

@raymondfeng
Copy link
Contributor

My take on TypeScript adoption:

Pros:

  • Productivity gain for LoopBack developers (optional type/interface checks, code refactoring, static verification, tooling)
  • State of the art JavaScript (async/await, decorators, ...)
  • More attractive to developers from Java/C# background

Cons:

  • Transpilation needed
  • Learning curve of another new language
  • Add complexity to support both TypeScript and Vanilla JS for extension/application logic

I'm open to trying it out with a few modules.

@superkhau
Copy link
Contributor

superkhau commented Jan 12, 2017

I agree with everything @bajtos + @raymondfeng said above. I still don't see the major reason for jumping to typescript in nodeland.

Pros:

Cons:

  • Unattractive to an (unknown ATM) portion of node users (how do we finalize on the language without knowing the popularity/usage numbers etc)
  • Transpilation languages like CoffeeScript haven't stuck in JS land period (ie. CoffeeScript like @bajtos mentioned above)
  • Need to add a build system to complicate things (gulp vs plain npm scripts) -- I did get some suggestions from a community member on gitter though regarding https://github.com/TypeStrong/ts-node which would alleviate dev time issues, but IMO it's still more complication in the mix)
  • I don't want to run into Angular 2/React situation where we force typescript down community's throats and narrow down our contributor market (ie. 100% node now to only TS contributors)

IMO, all this extra complication is not needed. I believe everyone using LoopBack wants to see feautres and extensibility for features we don't currently support OOB. TS may make us better (or worse), but that is not what is going to sell LB4. Supporting both TS + vanilla JS IMO is doubling our effort for what I'm going to say little gain.

Anyways, it all comes down to who we're selling to. Are we targeting only Java/C# background devs at the expense of vanilla JS devs?

Personally, I am quite interested in giving it a shot (new shiny toy syndrome). Just not sure about the long term ramifications for the LB project itself.

@ritch
Copy link
Contributor Author

ritch commented Jan 12, 2017

Skate to where the puck is going, not to where it is.
-Gretzky

There is something entirely missing from this thread... We are discussing the current state of affairs in node / javascript land. Remember and always consider this quote in the design of loopback-next. The release time frame we are talking about is ~ 6 months (much longer than the original release of LB btw, and about the timeframe of LB 2). How much can change in 6 months? Node.js went from a project that no one cared about to millions of users in that time frame.

A lot of the points above are arguments against things that no one is advocating for. Yes the class keyword is syntactic sugar for:

function Foo() { 
}
Foo.prototype.bar  = function() { 
}

(btw this is the first code in this thread about code... another thing to keep in mind)

My point is: syntax is not the focus here.

TypeScript - JavaScript that scales.

The idea is to embrace the ideals of TypeScript. Take this snippet from an email I just received from one of our customers:

Currently we have ~100 people developing product code with Node, Loopback, Express, Passport etc. By September that number will be close to 250.

Our customers need a framework that scales to dozens and even hundreds of developers. Its the reason TypeScript exists and is gaining traction. It is so much more than the class keyword.

Tooling

TypeScript users share the same IDE: Visual Studio code. This means that the LoopBack ecosystem could someday be filled with useful best practices around that IDE and even plugins that improve the dev experience. Right now that effort is split between various editors and basically non-existent.

Static analysis also makes more robust tooling possible. Its the foundation which this scalability comes from. The ability to easily refactor code without the common human introduced errors. Dev and Compile time checking. Think about the effort we have spent on liniting, Our users have this same problem but don't have the same expertise and time we do to setup complex linting solutions (eg. 1 linting config that works across many projects).

Best Practices

Large scale development teams are usually led by folks who understand traditional OO best practices. We might all agree that OO should die in favor of functional programming, but I don't think thats going to happen very soon (although I wish it would). LoopBack already has psuedo OO design baked into is DNA (see example below)

// JavaScript - now
var MyModel = loopback.Model.extend('MyModel', properties, options);

// TypeScript
class MyModel extends Model { // this removes a lot of ugly code in Juggler

Anyways there is plenty to discuss... and the discussion could go on forever. My plea is that we get deeper into the actual code so the conversation is less abstract. That way we can comment on our experience with type script, whether thats performance, cost vs benefit, etc.

@ritch
Copy link
Contributor Author

ritch commented Jan 12, 2017

Closing this for now. Lets revisit after the spike.

@ritch ritch closed this as completed Jan 12, 2017
@bajtos
Copy link
Member

bajtos commented Jan 13, 2017

// TypeScript
class MyModel extends Model { // this removes a lot of ugly code in Juggler

FWIW, this means that in order to create a custom MyModel, we must have access to the base Model class. The ways how to achieve that, which I am aware of, look too complex to me:

  • All components used by the application are depending on the very same loopback version (this is highly improbable).
  • Components need dependency injection allowing the application to provide them with the base Model class to use. This makes the example less simple and may break static code analysis/code completion:
    module.exports = (Model) => class MyModel extends Model {
    In fact, the type system may not even support a construct like this - how are we going to express that the exported method returns a type that's created inside the method and does not exist before the method is called? We also cannot provide type information for the Model argument, because the type depends on the application calling this code. (There may be other options for dependency injection in TypeScript though.)
  • Each component brings its own version of loopback. Both the framework and the applications must be prepared to handle this.

I was thinking quite a bit about this problem in the past, the solution I proposed for controllers (see https://github.com/strongloop-internal/scrum-loopback/issues/614#issuecomment-187124065) is to reverse the inheritance hierarchy:

// in a component or an app
class MyModel { // no dependency on anything from LoopBack yet
 // ...
}

// in LoopBack
app.model = function<Model>(ctor: Model, options: ModelOptions) {
  // build a subclass of ModelCtor that adds data-access methods like Model.create
}

This way, the components do not depend on LoopBack to implement their custom behaviour (business logic) and the dependency-versioning issue I described above is no longer relevant.

Of course, I don't whether TypeScript/ES6 classes are powerful enough to allow this construct. We may need to investigate different approaches as part of the spike.

The goal of my comment is to raise awareness of this problem.


One more thing to verify as part of the spike: are static methods inherited in ES6/TypeScript classes? In the current design, most data-access methods are static, i.e. MyModel.create, and inherited from base PersistedModel.

@bajtos
Copy link
Member

bajtos commented Jan 13, 2017

@ritch I think we all agree that users should be able to write their applications in TypeScript. The part where we do not agree is whether the framework itself must be written in TypeScript too.

Let's wait for the outcome of the spike to discuss this topic more.

@ebarault
Copy link

ebarault commented Apr 5, 2017

Hi, thanks for letting me join your discussions regarding LoopBack next with an early-preview ticket 👍

Given my experience of the framework as a contributing user for 2 years, and a community maintainer for 2 months. I don't see how you will humanly be able to carry on the support on LoopBack 2/3 in vanilla js and the development of LoopBack 4 in ts with sufficient velocity.

Why this total rebuild? IMO time would be more profitably invested on existing code reassessment to consolidate the existing features. Take for example what i'm trying to achieve with polymorphic relations and model inheritance.

I'm afraid that during this redesign more bugs will be added on top of existing ones and it will soon become unmanageable to identify the origins of the problems. On some features already I need to dig in 4 years of commit history to understand the stacking of logics contributing to a same feature.

Also, i take from other discussions in this repo that you wish to scale up through more community involvement which is great, but there's a huge risk of fragmenting the community here, not only because of the choice of this or that coding language, but because people have invested time to understand how the framework is built (including its subtleties) and how to contribute to it: I'd kindly suggest to move the framework step by step to keep active contributors on board. Progressive support for es6 class could be brought the same way promises were introduced in LB.

Remember these active contributors are also the same that use LoopBack every day to build applications with it and that will stay with LB3 for a while. Going with ts is like asking them to support bug-fixing and evolutions on both the vanilla js version and the new ts version. Huh huh...

Support for applications leveraging ts is a different story.

@bajtos
Copy link
Member

bajtos commented Apr 6, 2017

Thank you @ebarault for a thoughtful comment. Frankly, I don't know what to say - this decision was made mostly by @ritch despite my/our objections, see his #6 (comment)

Progressive support for es6 class could be brought the same way promises were introduced in LB.

I think adding support for ES6 classes to the current design of model/model-builder/model-registry is much more difficult than may look at the first sight. Right now, the model constructor is generated by model-builder and registered in model-registry. I think we would need to significantly change this part to support ES6 class inheritance, which may in turn break a lot of things across the codebase.

I am not saying it's not doable, it's just one of the pain points where a clean rewrite looks like an easier path than trying fix the existing codebase.

@bajtos
Copy link
Member

bajtos commented Apr 6, 2017

Skate to where the puck is going, not to where it is.
-Gretzky

How much can change in 6 months? Node.js went from a project that no one cared about to millions of users in that time frame.

FWIW, I did some benchmarking of native async/await in the upcoming Node.js 8.0.0 and native async/await is about 80% slower than vanilla Bluebird, which is still about 100% slower than pure callbacks. The numbers are for my particular benchmark, real-world code may perform differently.

The point I want to make that Node.js is no longer evolving that fast. We will have to support Node.js 6 for another two years and the next big thing, Node.js 8, still does not deliver a high-performant implementation of async/await and promises.

So yes, async/await is the future, but it won't be here for at least another year.

@ritch
Copy link
Contributor Author

ritch commented Apr 10, 2017

Hi, thanks for letting me join your discussions regarding LoopBack next with an early-preview ticket 👍

@ebarault thanks for chiming in. This discussion lacks a lot of follow up context, so please don't assume this is where the conversation ended. We did the aforementioned spike and started implementing some features. We are pretty far down this path and so far its very promising.

Given my experience of the framework as a contributing user for 2 years, and a community maintainer for 2 months. I don't see how you will humanly be able to carry on the support on LoopBack 2/3 in vanilla js and the development of LoopBack 4 in ts with sufficient velocity.

I share the same concern. This problem of efficient maintenance doesn't just go away if we don't go down this route though. This is an attempt to scale the folks who can contribute. If we can grow our contributors with a more approachable codebase - in theory it won't be a split of existing contributors like you are proposing. I'm much more interested in discussing if loopback-next is more approachable as a contributor. The rest of this sounds like point in time fear, which I totally understand.

Why this total rebuild? IMO time would be more profitably invested on existing code reassessment to consolidate the existing features.

There are some critical issues with the design of LoopBack core @3.x. This effort doesn't include rewriting/redesigning the juggler. This is a rewrite of the core codebase, router, registry, auth, and other features in core LoopBack (strongloop/loopback and strong-remoting).

Take for example what i'm trying to achieve with polymorphic relations and model inheritance.

I read this as: "I'm improving an existing feature instead of re-writing it from scratch?". If so, this is a great approach until you get to a point of diminishing returns. There are things we'd like to do that aren't practical given the current core (eg. DI, getting rid of loopback-boot).

I'm afraid that during this redesign more bugs will be added on top of existing ones and it will soon become unmanageable to identify the origins of the problems.

The idea would be that we start contributing new features on top of the new core codebase, focusing on fixing bugs in LB3.x. This shouldn't add to existing bugs.

On some features already I need to dig in 4 years of commit history to understand the stacking of logics contributing to a same feature.

To me this sounds like a problem this core code-base rewrite can potentially solve. It allows us to use a lot of what we learned over the past 4 years to build a solid foundation that is consistent and easy to understand.

Also, i take from other discussions in this repo that you wish to scale up through more community involvement which is great, but there's a huge risk of fragmenting the community here, not only because of the choice of this or that coding language, but because people have invested time to understand how the framework is built (including its subtleties) and how to contribute to it: I'd kindly suggest to move the framework step by step to keep active contributors on board. Progressive support for es6 class could be brought the same way promises were introduced in LB.

This misses the perspective that there is a huge pool of people out there that would contribute to LoopBack if it were less opinionated, simpler, faster, more flexible, etc. Of course there is vulnerability, but you can't have a real community without that.

Remember these active contributors are also the same that use LoopBack every day to build applications with it and that will stay with LB3 for a while. Going with ts is like asking them to support bug-fixing and evolutions on both the vanilla js version and the new ts version. Huh huh...

We have critical issues we want to solve with core. We want to add features but need to fix those issues first. We have to do this regardless of TypeScript.

I understand the argument of "existing contributors won't be so happy to support something they aren't using". I'd say to that, well we need to make sure loopback-next is useful enough for them to justify migrating.

@raymondfeng
Copy link
Contributor

raymondfeng commented Apr 10, 2017

For the record, here is my take:

  1. We choose to rewrite LoopBack core in TypeScript for a few incentives:

    • Productivity
    • Leverage latest and greatest or even future JavaScript constructs
    • Make more metadata explicit so that core maintainers can understand the code base better and easier
  2. We rewrite the core for much better extensibility/pluggability/flexibility in a consistent way across modules. The code base will be much simpler than LoopBack 3.x. The core will become a mini kernel with well defined extension points and a lot of responsibility will be shifted to extensions (which can be vanilla JS or TypeScript).

  3. We'll refactor loopback-datasource-juggler based on what we learn from 1 and 2.

  4. The application logic will be supported in both vanilla JS and TypeScript.

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

No branches or pull requests

5 participants