Open amcgee opened 5 years ago
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..
When 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 utils
directory. This still adds documentation of what data the dev has to deal with (which I think is the most important documentation value of typescript). I don't add return types for functions anymore unless they are intentionally specific, most functions should work on one type (like Object or Array) but not on concretions of these types. (This is the reason why the clojure community loves their built-in types, they're rich yet simple and when keeping to this rule of not introducing more concrete types, it reduces development time and makes functions being reusable in many more ways).
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.
TS has three very nice features:
implements
in an OOP sense, but still not the same)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 Promise
, Array
(like Array<Partial<User>>
and node module definitions).
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: (state, { action, payload } = {}) => ....
, it's easier to define the payload's type just where you're using the payload:
case SOME_ACTION:
return { ...state, users: [ ...state.users, formatUserInput(payload as AddUserActionPayload) ]}
I use union types mostly when having a type string
but really expect 2 or 3 different string constants.
type InputStatus = 'empty' | 'valid' | 'warning' | 'error'
I'm in favor of the "no implicit any" tslint rule, but using any
often helps in many situations. The example with the types payload above isn't really necessary, I stopped typing payloads and it works perfectly well (I just add types to the parameters of the action creator function), so I can just do:
case SOME_ACTION:
return { ...state, users: [ ...state.users, formatUserInput(payload as any) ]}
I 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):
I moved from a more design background to engineering, so I started out with css, html and js, and those have been the languages I use 95% of the time. Not starting out with a statically typed language might have led me to deal with developing a maintainable app differently. For me, a lot of the things that improve an apps maintainability in the frontend are soft skills: a clear design (as in goal of the app), discipline, proper architecture, etc. I find html, css and js extremely forgiving and that introduces a lot of potential footguns as you have to set the boundaries yourself. I however can't say that after applying the former properly that I've ever felt like I needed static types (besides of course proper tests, etc. as mentioned in the article as well).
In general I prefer as few abstractions between the code I write, and the code that eventually gets run on the client's device as possible. For me, adding an intermediary layer needs to really be worth it, as it's something that can introduce bugs, adds indirection, and needs to be maintained. Plus you run the risk of being stuck with a technology that loses traction and will have to refactor it out (see coffeescript).
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:
user && user.profile && user.profile.id
that might be a sign ;-) )My concerns are:
@babel/preset-typescript
has no benefits[1] from what I can gather.From the article in [1]:
Using the TypeScript compiler is still the preferred way to build TypeScript. While Babel can take over compiling/transpiling – doing things like erasing your types and rewriting the newest ECMAScript features to work in older runtimes – it doesn’t have type-checking built in, and still requires using TypeScript to accomplish that.
So even if Babel builds successfully, you might need to check in with TypeScript to catch type errors. For that reason, we feel tsc and the tools around the compiler pipeline will still give the most integrated and consistent experience for most projects.
As we mentioned above, the first thing users should be aware of is that Babel won’t perform type-checking on TypeScript code; it will only be transforming your code, and it will compile regardless of whether type errors are present. While that means Babel is free from doing things like reading .d.ts files and ensuring your types are compatible, presumably you’ll want some tool to do that, and so you’ll still need TypeScript.
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:
Small, modular greenfield (newly created) libraries
Mission-critical pieces of infrastructure, particularly when they need to present a robust, stable API which are consumed by multiple other components
A Large Scale Study of Programming Languages and Code Quality in Github:
Programming Errors. Generic programming errors account for around 88.53% of all bug fix commits and occur in all the language classes. The regression analysis draws a similar conclusion as of RQ1 (see Table 6) since programming errors represent the majority of the studied fixes. All languages incur programming errors such as faulty error-handling, faulty object and variable definitions, incorrect data initialization, typos, etc.. Still, some programming errors are more language-specific. For example, we find 122 runtime errors in JavaScript that are not present in TypeScript. In contrast, TypeScript has more type related errors, since TypeScript compiler flags them during development. Thus, although generic programming errors occur in all languages, some kinds relate to specific language features.
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 Flow is a static type checker (provides a type system) only, while, as far as I know, TypeScript is more than that. Flow definitely has its problems (obscure errors, breaking changes with each new release etc.), but I imagine TypeScript do to. TypeScript has a bigger community though.
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:
But I will not use the current version of TypeScript in my next large scale application, because the larger the project is, the more the costs of using TypeScript compound.
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).
It turns out that type errors are just a small subset of the full range of possible bugs, and there are other ways to catch type errors. The data is in, and the result is very clear: TypeScript won’t save you from bugs. At best, you’ll get a very modest reduction, and you still need all your other quality measures. Type correctness does not guarantee program correctness.
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:
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:
Once we have decided that we do want to use static types to build libraries/applications, we can start to evaluate specific technologies.
implements
, inherits
, ...) with different semantics?Further down the line, we should also consider:
N.B. the intent is not to get a "yes" on each point, but to gather facts so we can make an informed decision.
This 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.
Over the last week I spent offline-time thinking about the adoption of TypeScript
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
It'll create inconsistency across libs. Libs will be using TS, apps will be using JS. Should we port existing JS libs to TS? I think this will add a lot of work on top of what we already have to do.
We already have a lot to do And we should focused on getting that done (the things @varl mentioned in the comment above).
String types require more development + maintenance time To me types are basically unit tests for types at compile time, but testing logic is much more important than testing types, at least in my opinion. But having to spend extra time for tests + types while still having libraries that depend on other libraries, it means a lot of extra time you need to invest when working on an app and you realize you have to add/change something in d2-ui which requires you to change something in d2.
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
My main take-away from this is the Maintenance
and Puzzle
part. Devs (me included) start to solve puzzles they create themselves while developing and this happens especially with types (unless used very sparsely, then they add a lot of value, which I mentioned in a comment before, but that's something that usually gets out of control quickly) and after having introduced puzzles, maintenance of them will be painful.
This became somewhat stale..
After having worked on the data engine in app-runtime
I got some firsthand experience with typescript and dhis2. I think it's nice to have types and type-safety, but it was really difficult to change stuff. That relates to my past experience with typescript:
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:
If we provide functionality that expects an object as input argument, I think it's bad practice to restrict the allowed properties on that object, it's much better to just ignore everything the function doesn't need, which enables flexibility and composability (is that a proper English word??). With typescript, we'd have to define interfaces like this to enable that "protocol"-like behavior:
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
With typescript, we'd have to define interfaces like this to enable that "protocol"-like behavior:
This is not true. An interface
is always extensible while a type
is "closed", as you'd expect from the names. You don't need the dynamic parameter at all. The example code you provided, cleaned up a bit, is very nearly as succinct as pure JavaScript but has compile-time parameter checking for all function calls (unlike jsdoc) and code completion in VSCode and other editors (VERY helpful for discoverability by 1st and 3rd party devs).
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 (https://github.com/dhis2/app-runtime/pull/115, https://github.com/dhis2/app-runtime/pull/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.
To me typescript adds more complexity and problems than it solves [...]
Reasons broken down for readability:
- 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
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:
ensure working/bug-free functionality with an extensive test-suite (unit tests, integration tests & functional tests [...])
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.
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.
and
We will not be rolling out TypeScript outside of dhis2/app-runtime until we have made a formal decision to do so.
OK, so now we have two TypeScript projects (app-service-datastore
is new) without having reached a formal decision of expanding TypeScript beyond the app-runtime
, so for accountability we should deal with that head on.
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.
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.
and
We will not be rolling out TypeScript outside of dhis2/app-runtime until we have made a formal decision to do so.
OK, so now we have two TypeScript projects (
app-service-datastore
is new) without having reached a formal decision of expanding TypeScript beyond theapp-runtime
, so for accountability we should deal with that head on.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 app-service-datastore
was actually discussed between me and @amcgee in slack that it should(?)/might be integrated into the app-runtime
in the future, but now when it's not in a production-ready state we just temporarily moved on and released it as an alpha to be able to work with it more easily across different devs in apps that might use it such as the DQ Tool
@erikarenhill seconded, app-service-datastore
(as the namespace implies) is a candidate for a service in app-runtime
, a sibling of app-service-data
and app-service-config
. That is why it is written in typescript. If we don't move it into app-runtime
we can strip the types.
@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 app-runtime
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 dhis2/app-service-datastore
, though this is less important than the update here. It is OK if an experiment changes scope -- I've done that myself -- but we need to record the changes so they happen transparently.
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've used TypeScript in a few projects already, really love it,
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 :laughing:). 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.
I 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 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).
As 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
This 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)
As 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.
While 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
The 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
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. See my comment above, I'd be against introducing TS
@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!