TypeScript + Monorepo + Various fixes #144
Conversation
| "split": "^1.0.1", | ||
| "xo": "*", |
There was a problem hiding this comment.
Do not change our code style.
There was a problem hiding this comment.
It was a way of updating the linter to fit the current standards and ecosystem. XO has 30k downloads vs eslint/prettier who have more than two million. A customized eslint setup with prettier and typescript-eslint is much more powerful and flexible than xo. Let's use whatever coding styles you like, but don't use outdated and unused tools for linting when you have a clear industry standard (eslint) that provides by far the best DX, features, and IDE integrations.
There was a problem hiding this comment.
Sorry I can't agree with you. XO is based on ESLint. It is inappropriate for you to try to compare them. We can continue to use XO's code style, and it also supports TypeScript.
Please do not remove AVA and XO, we do not want to change in this regard. Sam and I use AVA and XO for all our open-source work and we like them.
There was a problem hiding this comment.
Ok, feel free to change it. Just so you know I doubt XO has as good typescript rules as typescript-eslint. And AVA sure lacks a lot of features jest has to support this type of typescript monorepo. I wouldn't stick with them for too long if I was you.
|
@SamVerschueren @LitoMore any comments on this? |
|
I'll try to review it shortly. Thanks for this massive PR :). |
Based on the conversation at #143
This PR is a complete rewrite in TypeScript using lerna to include the 3 main renderers and jest/babel for testing. It also introduces a new package,
listr-corewhich has the classes and types shared betweenListrand a renderer.The build results contain the type definitions, so there will be no need for
@types/listranymore.One of the main issues while using listr was its dependency on RxJS, that has been removed and replaced by a barebone Observable + Observer implementation to preserve the spirit of the base code as best as possible while also minimising changes to the Task-Renderer interface.
Some bugs/odd behaviours have been fixed and they will affect end users and should be considered breaking changes, although they were strange and buggy behaviours. You can see these by looking at the differences between the old tests and the new ones
There are also breaking changes in the Task - Renderer interface, so any custom renderer out there will have to update to the new interface and inherit from
BaseRendererinlist-core.Code coverage is not reflected properly as
listr-coreis mostly tested via tests inlistrthat use the builded.jsfiles and not the actual source code, so the metric itself is not representative right now. All the tests have been preserved and even some additional checks have been added.i've set lerna in singl-version mode bumping all packages to 0.15.0, but that is of course subject to change. I thought it was convenient to publish all the packages at the same time with the same version.
No features have been added on purpose, to limit the scope of the PR.