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

Add flow/typescript toolchain/workflow #2702

Closed
chabou opened this issue Feb 24, 2018 · 13 comments
Closed

Add flow/typescript toolchain/workflow #2702

chabou opened this issue Feb 24, 2018 · 13 comments
Labels
💬 Feedback Wanted Issue or PR needs input from the community! Lend your thoughts ✨ good first issue Issue is good for Hyper newcomers help wanted Contributions wanted towards the issue

Comments

@chabou
Copy link
Collaborator

chabou commented Feb 24, 2018

We really want to add flow or TypeScript in our project.

First step is to add it to our dev workflow: add dependencies, linter compatibility, automatically run before push...

Please help us to have some benefits of typed system.

@chabou chabou added help wanted Contributions wanted towards the issue good first issue Issue is good for Hyper newcomers labels Feb 24, 2018
@albinekb
Copy link
Contributor

I think it would be better if we used TypeScript since it has more types available and specifically xterm which Hyper is now heavily dependent on is also written in TS.

@jcrben
Copy link
Contributor

jcrben commented Mar 4, 2018

+1 for TypeScript. TypeScript seems more widely known (I use TypeScript, have not yet used Flow) and seems to have more momentum. It can also pick up jsdoc...

@rauchg
Copy link
Member

rauchg commented Mar 4, 2018

I'm ok with either as long as we have an incremental adoption story. For example, if we can start replacing one .js file at a time with one .ts file, then we can incrementally make lots of little PRs to get to where we want to be.

So far, the appeal of Flow to me has been that incremental adoption strategy. If we can do that with TS, I'm game

@chabou chabou added the 💬 Feedback Wanted Issue or PR needs input from the community! Lend your thoughts ✨ label Mar 5, 2018
@chabou
Copy link
Collaborator Author

chabou commented Mar 5, 2018

No problem to have incremental adoption with TS.
If there is no bonus to use flow (to be compliant with other Zeit's projects, for example), I think that @albinekb has a point: Maybe better to use TS because xterm uses TS.

I let this issue opened to let other people add their inputs. I suggest to definitely choose our type system in 7 days.

@chabou chabou changed the title Add flow toolchain/workflow Add flow/typescript toolchain/workflow Mar 5, 2018
@Tyriar
Copy link
Contributor

Tyriar commented Mar 5, 2018

Re-adding my thoughts here from Slack. xterm.js was originally a 5k+ line file and separating it out into different files was one of the main reasons to use TS. I incrementally pulled modules out into their own files while converting, you can check what these sort of PRs look like with the linked PRs on xtermjs/xterm.js#335, though it will certainly be easier with Hyper as you already use a newer style of JS.

@saadq
Copy link

saadq commented Mar 12, 2018

I'm a user of both TypeScript and Flow. For what it's worth, the xterm.d.ts defs are pretty simple and it would be pretty trivial to write a Flow libdef from it (just taking a glance, I think the file is already 99% compatible with Flow). So if that's the deciding factor, I could submit a PR to flow-typed with a libdef for xterm.

With that said, TypeScript is definitely in the lead when it comes to tooling, community size, and developer support. The Flow team seems to not have too much time to deal with GitHub issues unfortunately, so the support you get from the repo will mainly be from Flow users rather than the Flow team itself:


I haven't been using TypeScript that long, so please correct me if I'm inaccurate about anything, but here is just what I've noticed after having used both:

Pros of Flow:

  • Easier to add in to an existing babel build system (at least until Babel v7 with babel-preset-typescript)
  • More type inference
  • The ability to run flow coverage to see which lines of your code are covered/uncovered and you can also use flow-coverage-report for a general overview
  • Nicer error messages (Flow used to be notorious for it's weird error messages, but as of v0.66, they have been nicer than TypeScript's errors in my projects, even with ts pretty output)

Pros of TypeScript:

  • Much larger community size
  • Way more library definitions
  • The TypeScript team is very active in helping out users and responding to their GitHub issues
  • Works great with VSCode (There is a flow package for VSCode which works well, but the native TypeScript support is on another level)

With all that said, I prefer Flow, but I think TypeScript might be a better choice due to the amount of libdefs/community available. However, I don't really know much about hyper's codebase, so I guess keep that in mind.

Also worth checking out: Why Reddit Chose TypeScript. That article seems to say the same thing – they preferred Flow's type-checking and simpler setup, but they went with TypeScript due to its ecosystem.

@markozxuu
Copy link

@chabou @albinekb any update on this? in case we choose flow or typescript I would like to help in the migration.

@saadq
Copy link

saadq commented Apr 2, 2018

As a side note, I submitted a PR to flow-typed for xterm here: https://github.com/flowtype/flow-typed/pull/2017/files

Just so that it isn't the deciding factor. Either way, I'd also be happy to help out if I can in the migration.

@Tyriar
Copy link
Contributor

Tyriar commented Apr 2, 2018

As a side note, I submitted a PR to flow-typed for xterm here: https://github.com/flowtype/flow-typed/pull/2017/files

Just so that it isn't the deciding factor.

@saadq this is a little different; the xterm.js API changes pretty much every month and it's source of truth is the TS declaration file shipped with the module. It also appears your editor would lose all the jsdoc info that goes with it when using https://github.com/flowtype/flow-typed/pull/2017/files#diff-160b1966f9630834a6d6fc0154b529d0?

image

@saadq
Copy link

saadq commented Apr 2, 2018

Adding the comments to it would be trivial, but yes the fact that the TS typings is provided by xterm itself is definitely a bonus.

This was referenced Apr 23, 2018
@Stanzilla
Copy link
Collaborator

PR: #2717

@LabhanshAgrawal
Copy link
Collaborator

Hi, I have opened a pull request #3816 adding TypeScript support.

@Stanzilla
Copy link
Collaborator

we're settled for TypeScript now, contributions welcome for porting parts over! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💬 Feedback Wanted Issue or PR needs input from the community! Lend your thoughts ✨ good first issue Issue is good for Hyper newcomers help wanted Contributions wanted towards the issue
Projects
None yet
Development

No branches or pull requests

9 participants