-
Notifications
You must be signed in to change notification settings - Fork 8
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
TypeScript for static typing #33
Comments
I've used TypeScript in a few projects already, really love it, but have to say that there are some bad practices which are used by the majority of TS devs, which most of the time is a result of not having enough experience with it. At least this is my experience with TS.. Bad practice 1: Add types for everythingWhen introducing types, devs tend to add types for everything (that happened to me as well), like function definitions, redux store interfaces, every little piece that would've been a class in OOP. This makes it very, very hard to refactor code and will probably introduce a lot of debugging types during development (and that can be a PITA!) I started to create only interfaces and types for data, that will be stored in the redux store but will also be used by functions in the This brings me back to the Comment about function argument objects as a protocol vs schema. Introducing types often results in functions expecting a certain schema rather than providing a protocol of what they expect, which will end up in very thight coupling of types and functions while refactoring will become very time consuming as types will have to be refactored as well. Bad practice 2: Complex typesTS has three very nice features:
You can do a lot with these three features and I REALLY love them, but they should be used only when it really makes sense, not just because you can do it. 90% of generic types are usually for Union types are often misused as well, like dealing with redux action payloads. Providing the action type in a reducer's argument list is impossible (well.. possible.. but not good..). When destructing an action in the reducer's argument list (like so: case SOME_ACTION:
return { ...state, users: [ ...state.users, formatUserInput(payload as AddUserActionPayload) ]} I use union types mostly when having a type type InputStatus = 'empty' | 'valid' | 'warning' | 'error' Bad practice 3: Being scared of anyI'm in favor of the "no implicit any" tslint rule, but using case SOME_ACTION:
return { ...state, users: [ ...state.users, formatUserInput(payload as any) ]} ConclusionI really like typescript and I'm in favor of using it, but it has a few drawbacks if some devs have little or no experience with it, we'll have to pay close attention to where types are added/being used, type complexity, etc. Maybe you guys see things differently, curious about the other opinions :) |
I think my opinion's based on several things (besides the points made in the article):
Hopefully that explains a bit where my opinion comes from. There's quite a lot to write about a topic like this, but I think at the moment these are the main points for me. |
I've only worked with typescript for a couple of months in an Angular 2 project. On balance I would say it was a pretty positive experience. I liked that it does actually catch stuff at compile time that I had overlooked. Mainly stuff like, function expects a userId, but I pass it a user. I also liked the fact that you can autogenerate documentation from your code with typedoc, which is actually pretty good. What I liked less, but this could be related to my personality, was the fact that I found myself spending quite a significant portion of my time designing the types. Even though TypeScript is all about "opting-in" and "incremental adoption", when I started using it, I felt I had to take full advantage of it. I did feel like I was making progress pretty slowly. But I guess that would become less so over time.... In conclusion, my views are similar to @amcgee:
|
My concerns are:
From the article in [1]:
I think that adding TypeScript is premature, and that we should focus on other improvements before considering it. I strongly feel that our are efforts better focused elsewhere, e.g. on improving our automated test situation rather than adding static typing at this point in time. Even if we limit where we introduce TypeScript, it will take focus away from other technical work like writing tests. We aren't at a level where tests are second nature to us, so layering another technology into our stack before that is a premature optimisation. Another aspect is that if we want to add static typing to our code we need to evaluate Flow as well because of the fact that at least one application uses it already. [1] https://devblogs.microsoft.com/typescript/typescript-and-babel-7/ |
Some data for the following situations:
A Large Scale Study of Programming Languages and Code Quality in Github:
Digging into the details related to TS and JS for the domains we are thinking about (Libs and Frameworks) TS indicates a slightly lower bug density, more so in libs than in framework code: |
A small book which was useful to me about TypeScript: https://basarat.gitbooks.io/typescript/content/ To clarify, I have no technical objections about TypeScript itself, I think that if we are going with types, that would be my preference, too. Adding the questions I think should be considered in the meeting to the agenda (#34) in a comment. |
So, we (the tracker team) are using Flow in the capture app. I decided to go with Flow for this project in late 2017, after I used it in my previous non dhis2 project (and was quite happy with it). I’ve never used TypeScript, so all my opinions are from a Flow standpoint. From the top of my head, these are the the most important benefits/concerns to me when using a static type checker: Benefits
I understand jsdoc is helpful here (and others), but a type system is even better.
Concerns
Flow vs TypeScript If we’re going to standardize on one, I think someone should look into both before making a conscious decision (If that hasn’t already been done). If this decision turns out to be TypeScript that’s fine with me. Whatever the outcome, I would like to continue using flow in the capture-app until we have a good alternative in place (like TypeScript). Just stripping the app of flow code would really hurt our productivity. PS! I agree with @Mohammer5 regarding the bad practices he mentioned. |
I still fail to see why we'd implement typescript and not focus on improving our process and other, more important, aspects of our codebase first. To quote the article:
Using typescript comes at a cost. The benefit of using typescript is negligible when compared to the benefits of having properly tested code, a proper process where it's clear for everyone working on the code what the goals are, etc. Whereas using it means more time spent onboarding new developers, an even smaller pool of devs to hire from (about 20% less), adding another tool into our already sizeable toolchain, more syntax noise (see article).
Why would we add this type of overhead to our already sizeable workload. I think we have a lot of other steps to make before I personally would even consider adding typescript to our projects. |
Over the last week I spent offline-time thinking about the adoption of TypeScript and I came to the conclusion that the discussion has begun in the wrong end, and as a result, is all over the place. There are many excellent ideas and points being raised on everything from how we can introduce a specific technology in limited use cases to if it is something we should do at all. It's great that we are having these discussions, and especially great that they exist over time outside of Slack. Great arguments aside, instead of starting the discussion around the concepts, we have jumped straight into discussing a specific technology which is the classic developer trap. In addition to that, the proposed technology is a new programming language for Front-End (which just happens to overlap with JavaScript ... for now). It is not a decision we should take lightly, and it needs to be considered as if the proposal was to use Elm. Let's not fall into the "Well, JavaScript is just like Java, right?" trap employed by nefarious marketers at Mozilla. We should absolutely discuss and evaluate it, but ultimately I reserve veto rights on that decision. Based on all of the posts in #34 and #33, it seems that we are not ready to make an informed decision to introduce another programming language into our stack, and I don't see how we can be by Thursday. I want to defer the technological debate, and approach the broader issue of static types and how to introduce them to our code base of 280K lines JavaScript, in a tiered approach. First of all, we need to take a step back and consider: What is the most important area for us to focus on right now?
Most of these are already areas we are working on but haven't completed. Should we add another to that list? If yes, we should evaluate the concept in relation to us and our code before looking at technologies: Static types concept
Once we have decided that we do want to use static types to build libraries/applications, we can start to evaluate specific technologies. Static types technology
Further down the line, we should also consider: Developer centric
N.B. the intent is not to get a "yes" on each point, but to gather facts so we can make an informed decision. Going forwardThis impact of this issue is to such an extent that we need to set up a task force of developers (preferably with cross-team representatives) to go through the tiers and provide a report to the other developers, which we can use as base to evaluate and determine if this is something we want or not. This would go in as an Epic in JIRA and then planned into an milestone as it would require multiple developers to go deep and focus on it over a period of time. |
I've spent time thinking about this as well and - although I really love working with TS - I think it's the wrong decision for us, you mentioned many of the arguments against it already, here's a list of why I think we shouldn't use typescript
Richard Hickey made some excellent points (in his abstract way of talking) when he was talking about dynamic types in clojure and why he thinks static types would be adding more problems than it's solving: https://youtu.be/2V1FtfBDsLU?t=3965 |
This became somewhat stale.. After having worked on the data engine in
Of course this is highly subjective: To me typescript adds more complexity and problems than it solves (increased maintenance and refactoring effort; higher barrier for new developers, frustration due to having to deal with type errors during development, required degree of experience with TS to be able to produce maintainable types, deviation from the JS tech stack). I'd rather ensure working/bug-free functionality with an extensive test-suite (unit tests, integration tests & functional tests; which we should have anyways) than adding another layer of complexity. I remember that @varl found it quite nice to see the types when reviewing a data engine PR, and I agree that having types is nice. But we don't need typescript for that, jsdocs is equally capable of documenting types. UPDATE: THIS IS NOT TRUE, I was mistaken here: Another reason why I think typescript is not suitable for our libraries: interface Args {
requiredArg1: string;
optionalArg2?: boolean;
[x: string]: any;
}
const aFunction: (args: Args) => string = ({ requiresArg1, optionalArg2 }) => 'foobar'
// If we didn't include the [x: string]: any;, then we'd get
// a type error in TS
// no problems whatsoever in JS
// with this call:
aFunction({ requiredArg1: 'test', baz: 'irrelevant' }) It seems that, unlike with pure JS, typescript is not meant to be used that way |
This is not true. An interface Args {
requiredArg1: string;
optionalArg2?: boolean;
}
const aFunction = ({ requiredArg1, optionalArg2 }: Args): string => 'foobar'
// Totally fine, no type error in TS
aFunction({ requiredArg1: 'test', baz: 'irrelevant' }) |
Ah really! Weird I remember it being not this case.. I must've gotten confused then.. I'll update my comment above. Regardless of that, the other things I still "apply" from my POV |
I also would like to add the data point that I was able to completely refactor the data service (dhis2/app-runtime#115, dhis2/app-runtime#127), decoupling the engine from the react interface, in about one day of effort BECAUSE types were well-defined. If the types had been in JSDoc, for instance, they would likely have gotten out-of-sync somewhere. It would have been a much longer and more error-prone process to do the same without Typescript, particularly when it came to updating and expanding the test suite during that effort. |
I maintain that use of TypeScript in DHIS2 web apps and libs is limited to the dhis2/app-runtime as an experiment, which is monitored so that we can hold ourselves accountable to it.
Reasons broken down for readability:
All important and valid points! It is also important to note that both @amcgee and @JoakimSM has quoted that they feel that types has made refactoring safer and faster. That said: The application runtime is a critical piece of the application platform where a type issue can bring down all the applications that use it-- and we intend to use it for all of our apps --so while I wholeheartedly agree with this:
The runtime is one place where having an additional layer of safety could be worth it. We will not be rolling out TypeScript outside of dhis2/app-runtime until we have made a formal decision to do so. |
and
OK, so now we have two TypeScript projects ( It's been more than a year since this discussion started (March 2019) so we should be able to pull out some conclusions from how TypeScript has worked out for us so far in app-platform and make an informed decision if we want to keep using TypeScript or remove it from our code. |
The |
@erikarenhill seconded, @varl yeah I understand this could be considered a loophole, and we should definitely evaluate the typescript experiment 1 year out, but I don't think stripping the types from a new (experimental) library should be high on our priority list? I wrote this originally in the Sri Lanka Contact Tracing app with the intent of extracting it and including in Worth discussing at least briefly, thanks for bringing it up |
@erikarenhill & @amcgee: regardless of where the discussion happened, an update to this thread is appropriate. There could also be a caveat in the readme for Technology creep is as real as scope creep, and has a long tail. Even more so when we are talking about a programming language level tech. @amcgee no, stripping types now is not a priority, but keeping records of communication and decisions in a less ephemeral place than Slack is. |
I think whether typescript offers benefits is beyond any doubt. The question whether it's worth being added to a project is highly subjective. My opinion has changed quite drastically (initially I thought that not using typescript would be just a mistake; boy was I wrong 😆). Everything I will state now is basically derived from my personal experience. I can see that other developers have different experiences and therefore arrive at different conclusions, so I'm fine with both adding TS and leaving it out. So I'm on the let's-not-introduce-TS-anywhere-else side. Typescript helps refactoring is an indicator that something else is missingI can see why this is an intriguing argument. From my perspective this means that other, required, things are missing, like proper documentation, e2e tests, integration tests (mostly for libraries; as these guarantee that a complete overhaul exposes the same api and functions the same way when used, regardless of how it works internally), (js|ts)docs. These are all required, regardless of whether we use TS or not, and should help to understand both the flow of the application as well as input parameters and return values. The time it safes is insignificant to the time it costsThe smaller a library and the fewer dependency a library has, the more this argument is irrelevant. But the bigger the app/library, the more frequently did I encounter situations where I had to debug TS errors, add missing types for dependencies that lacked types or spent a lot of extra time when refactoring a huge portion of code and its types (the non primitive types). It does not guarantee type-safety during the run timeAs more or less all operations within an app/library depend on the arguments given to it or returned by a third party source of information, the exact procedure is just a derivative of the input params / third party source information. So having TS in place, doesn't protect the app from that. In fact it's more like unit tests for types at compile time, while I think the problem is rarely the wrong type but more generally sime form of wrong input, which is why I think this should be covered by tests, not by TS It "steals" time from more important things that we could doThis is not really an argument against typescript itself but about the hierarchy of problems in regards to their severity / amount of harm they can cause. The most critical issues don't stem from not having type checks, but from completely different issues. I think that we should focus on other things like more communication, cross-team developer inclusion, creating specification/written expectations and generally we should include more developers in the decision making (the last point has worked quite well lately; the past couple of months; but(!) not across teams. UI is more or less maintained by team-apps only) Consistency & onboardingAs long as we use both TS and JS onboarding will be harder for the TS libs/projects. People with less/no TS experience have no real chance at participating without putting in some effort to understand both the project and TS. Understanding TS from it's technical capabilities doesn't mean you can automatically use it properly as designing types is a skill in itself. The more types you have, the harder they are to maintain, the more complex the projects gets (more files, more folders) which is even less ideal for open source projects that we'd love developers that are outside of our reach to contribute to. Lacks support for fully functional programmingWhile this is not really an issue at dhis2, writing code in pure FP style is very hard with TS (try to type a pipe/compose function that will tell you that one of the functions in the chain will receive a wrong type from the one before). Some types are possible to be declared but they're anything but readable I can't remember the last time I had an issue because of the wrong typeThe headline says everything, but I hardly have issues with the wrong type. It doesn't mean that haven't encountered wrong inputs, but the issue wasn't the type itself Abstracting types adds extra burden on the developer(s)To me, it limits the amount of abstraction I can add to code as a portion of that will go to the types. This does not happen on a technical level but inside the head when I have to keep various things in mind when working on code. Does that mean I'd remove typescript from the app runtime? Def. no, it's already there and adds some benefits. It's a fairly small library with zero dependencies so the overhead in general is very low. But I do think that consistency (especially for on-boarding) is key when it comes to new apps/libs or existing JS projects. |
I can see that there's more typescript now: https://github.com/dhis2/app-service-datastore I'd prefer to discuss this first before we decide if we actually want to go that way. |
@Mohammer5 yes, the comment you responded to was referring explicitly to this library (which is actually a service for app-runtime where we have explicitly allowed typescript) |
Ah, I see, fair enough |
This topic has come up a few times, so let's discuss it here.
From #31 :
@ismay
@Amcee
@ismay
@erikarenhill
Replying to @erikarenhill
I don't think that would be worth the effort. I'm in favor of Typescript for better type safety in the following situations:
This requires well-designed and easy-to-use tooling for stripping and generating types for libraries and apps. Once that's in place, it should be low-friction to add types where they help to clarify an API.
The main reason I'm pro-typescript is that it adds type safety and makes code more self-documenting (rather than layering documentation on top of the code and risking drift as the codebase changes)
@ismay I read that article too when it was published and agree with a lot of the points, but I think he undersells the benefits of static typing. Will re-read and respond more tomorrow.
I've included most (or a lot of) FE devs so we can invite broader swath of opinions. Feel free to un-request yourself if it's too much chatter. May the odds be ever in your favor!
The text was updated successfully, but these errors were encountered: