search

JunoLab / atom-julia-client

Juno a good IDE?
http://junolab.org
MIT License
273 stars 72 forks source link

Typescript init #705

Closed aminya closed 4 years ago

aminya commented 4 years ago

Ready to get merged

Please merge this so I can continue with other PRs

This PR doesn't change any behavior of Julia Client.

Initialization for Converting codebase to TypeScript:

This is the initialization of that PR.

I am distilling https://github.com/JunoLab/atom-julia-client/pull/704 into smaller PRs, so I can track the changes easier, and making reviewing the actual editing part easier for you.

All the code is stored in (lib_src) and doesn't change any source which is in use.

Config

aminya commented 4 years ago

@pfitzseb @aviatesk Could you please merge this PR?

aminya commented 4 years ago

I even simplified this PR more. I will do the whole process file by file in separate PRs, so you guys can continue developing without any conflicts.

So please merge this PR as soon as possible. This is blocking me from continuing.

aviatesk commented 4 years ago

Hi @aminya, sorry for taking a time to get back to you.

We would very much welcome JSfying of our code base (in per-file basis), but I really don't appreciate conversions to TypeScript.

The main reason is that we're planning the migration of ink and julia-client in the coming months and it will be such a big refactoring. At this point we don't want to give the other huge change to our code base but rather keep our code base as it has been, in order to perform the migration safely.

TS seems more promising for current front-end development in general, and it would be fancy to have type-safety and more extensive development utilities for Juno, in the future. But this is clearly not our priority for now. And to say more, I'm not sure TS will really help our code base, at least as far as we live with Atom:

aminya commented 4 years ago

Hi @aminya, sorry for taking a time to get back to you.

Hi! No problem.

We would very much welcome JSfying of our code base (in per-file basis), but I really don't appreciate conversions to TypeScript.

The main reason is that we're planning the migration of ink and julia-client in the coming months and it will be such a big refactoring. At this point we don't want to give the other huge change to our code base but rather keep our code base as it has been, in order to perform the migration safely. TS seems more promising for current front-end development in general, and it would be fancy to have type-safety and more extensive development utilities for Juno, in the future. But this is clearly not our priority for now.

Regarding this ink and julia-client merging, I have talked to @pfitzseb, and we said that it worth it to convert in the merging process. I would be happy to help merging Ink, but I think Julia-client itself can be enhanced on its own without mering Ink.

And to say more, I'm not sure TS will really help our code base, at least as far as we live with Atom:

* type-safety provided by TS isn't strict/true -- the case is much worse especially when we have lots of dependencies that don't provide (reliable) type definitions (like etch for our case)

* Atom doesn't support native TS compiler, and so TS will add another complexity for our development process

* lots of development utilities provided by TS-server are available for JS files

I already talked about the many benefits here: https://github.com/JunoLab/atom-indent-detective/pull/17#issuecomment-599954980, but I didn't get any response from you:

  • You don't need to do anything regarding TS. This package is good to go and will act like a JS package for usage (JS is already built).

  • Typescript isn't about just tracking the implementation. It brings many improvements over simple JavaScript (enhances JS without drawbacks):

    • it reveals many things that are not visible when you use JS. Things like type-changing of the variables, if a function returns the type we want, performance improvement by using better types, etc. You see this package loads 1/4 of the original, and its runtime was better in my measurements (30-50%).
    • IntelliSense/linting
    • API documentation on the fly. It allows others to understand what the code is doing.
    • You can write your normal JS code in TypeScript and it works. TypeScript infers many of the types automatically.
    • It removes the need for babel or any other processor.
    • We can compile TypeScript to Wasm using AssemblyScript which opens a whole new world for improving the package. For example, I plan to compile the getIndet function of this package.
    • The future isn't plain JavaScript. For example, VSCode is already using TS (which IMO has better performance while I like atom more).

Read this article to know more about the advantages: https://dzone.com/articles/what-is-typescript-and-why-use-it

Last point: Since I am doing the work for you (in a systematic way that is easy to follow), I don't really understand the reason for the objection and resistance.

aviatesk commented 4 years ago

I'm afraid to say, but did you really read my opinions ? What do you think on them ?

  • type-safety provided by TS isn't strict/true -- the case is much worse especially when we have lots of dependencies that don't provide (reliable) type definitions (like etch for our case)
  • Atom doesn't support native TS compiler, and so TS will add another complexity for our development process
  • lots of development utilities provided by TS-server are available for JS files

You should always try to see pros and cons of things. And since this change is really big one, I would like to figure out it is really necessary to be done.

aminya commented 4 years ago
  • type-safety provided by TS isn't strict/true -- the case is much worse especially when we have lots of dependencies that don't provide (reliable) type definitions (like etch for our case)
  • lots of development utilities provided by TS-server are available for JS files

I mentioned the answers to these in my points: TypeScript is just JavaScript. lack of type definitions doesn't break the code or doesn't mean that it is not compiled. It just means that the code will be copied (compiled) like it is a simple JavaScript.

The reason that you think this adds complexity is that you think I am changing JavaScript to C++ for example 😄 while I am just fixing the code issues itself.

P.S: I have already made a PR to etch. If they don't merge it, I will instead make a @types/etch in a day: https://github.com/atom/etch/pull/82 While we don't need this because of what I just said in the above paragraph.

  • Atom doesn't support native TS compiler, and so TS will add another complexity for our development process

There is a compiler: https://github.com/smhxx/atom-ts-transpiler. But we don't wanna rely on the user's computer for compiling anyways. It makes sense to deliver the ready JS files to the user instead.

aminya commented 4 years ago

I separated Github Actions part of this PR into another PR https://github.com/JunoLab/atom-julia-client/pull/712, the failure of the linter on Travis is because of this. After Github's actions merge, the CI will pass.

aviatesk commented 4 years ago

So after all, I would like to decline you suggestions for migrating into TS. Reasons: