[HN Gopher] Bad TypeScript Habits ___________________________________________________________________ Bad TypeScript Habits Author : jgwil2 Score : 68 points Date : 2021-02-02 19:22 UTC (3 hours ago) (HTM) web link (startup-cto.net) (TXT) w3m dump (startup-cto.net) | SmellyPotato22 wrote: | I disagree with claim that One letter generics are a bad habit. | In my opinion, it is proper to use one letter generics but start | with A then B, then C... so it enables to reason about generics | positionally. Pseudo example of this: | | `map<A,B>(fn:(el:A)=>B):B[]` | brundolf wrote: | Hard disagree. Having come into a TS codebase from the outside | that made liberal use of generics, they can make things | incredibly difficult to follow if they don't have really good | naming and/or docs. | Garlef wrote: | Yes. But do long names even help? I imagine, in such a | project the system of generics can be so complex that a new | dev cannot use them to their intent without completely | understanding the code. | brundolf wrote: | You could say the same thing about variable names: | | "But do long names even help? I imagine, in such a project | the system of logic can be so complex that a new dev cannot | use them to their intent without completely understanding | the code." | | Meaningful names _help_ a person gain an understanding of | the code. And that 's not even getting into whether or not | "completely understanding the code" is a tractable goal in | the first place (I don't think it is, even for the people | who _wrote_ the code, much less those who come later) | Kranar wrote: | Long variable names don't help either. It's been mostly a | stylistic choice and a recent trend that stylistically | long variable names are nice. But there are plenty of | projects that use short variable names or even one letter | names, and I don't see any evidence that those projects | are less productive because of it. | | The actual research is that long variable names make it | harder to understand code. It makes the variable name | become an integral part of any code snippet that a reader | feels compelled to memorize the specific names used, and | that memorization results in errors and increased recall | time. | | Short variable names, including single letter names, | allow a reader to basically ignore the name itself which | allows them to accurately memorize an entire chunk of | code without feeling the need to memorize long names. | This improves recall time. | | In simpler terms, when a snippet of code uses descriptive | variable names your brain is inclined to believe that | those names matter and that you need to remember them, | compared to short variable names that your brain will not | have to bother remembering and instead frees you up to | focus on the structure and semantics of the code snippet. | | The study that you can read, which is quite insightful is | here: | | https://pdf.sciencedirectassets.com/271600/1-s2.0-S016764 | 230... | | Note that long function names DO help, but long variable | names do not. | | That said, feel free to use long variable names if that's | what you like and your project does it that way. Don't, | however, express that as some kind of extensively studied | software engineering principle that has research backing | it. It's basically one of those things someone decided | was true and everyone else just decided to believe it | without ever doing the kind of work needed to validate | the hypothesis. | | In software engineering, we are still in the days of | Aristotle, where we simply believe things to be true | because we want them to be true. Just like the days of | Aristotle I wouldn't be surprised if it takes us | literally thousands of years before someone comes along | and decides to question and test basic assumptions we all | take for granted. | jgwil2 wrote: | I think I agree with you (though I would favor T, U, V as the | sequence). | | What would the alternative look like in what you wrote? In and | Out? Range and Domain? Generic type variables are often too | abstract to give a sensible name to. | | This is the one "bad habit" that I think is a bit silly - | naming conventions can vary by project. | SmellyPotato22 wrote: | In the context I am thinking about, writing a generic | iterator, you have 0 context of the what the concrete types | will ever be. | MrPatan wrote: | Do you also use one letter variables and parameters? If not, | what is different about generics? | | Even in your example, I'd rather read this: | | map<IN,OUT>(fn:(el:IN)=>OUT):OUT[] | | Plus, it really is a habit. If you stop doing it for these kind | of examples that really are very generic, maybe people wouldn't | also use only one letter for cases where there is more of a | difference between them. | SmellyPotato22 wrote: | Kinda gets a little hairy if you start to add more types. | Example: `//fork :: [a] ->((a -> b ), (a -> c)) -> ([b] | ,[c])` | | `fork<A,B,C>(fn1:(el:Iterator<A>)=>B,fn2:(el:Iterator<A>)=>C) | ` | MrPatan wrote: | What would be the opposite of "fork", something like "zip" | that gets two lists and combines them into one? Would it | have a signature also starting like this: `zip<A, B, | C>(...` | legerdemain wrote: | > Do you also use one letter variables and parameters? | | A question in response to your question... Do you ever give | an ALL CAPS name to a class? Why not? | | Maybe it's the set of conventions I'm used to, but I find T, | U, V familiar and IN, OUT jarring and dramatically less | readable at a glance. | MrPatan wrote: | I use ALL_CAPS in generics so they are easy to tell apart | from functionNames and ClassNames | Kranar wrote: | Sometimes I do, sometimes I don't. If it's an iterator I | often use one letter names such as i, j, k. Similarly if it's | a generic unbounded type I also give it a one letter name. | paxys wrote: | Nice article, but all the negatives made it a little hard to | read. For some points it was hard to figure out whether the | author was in favor of doing or not doing it. | bengalister wrote: | About #2 Wasn't so sure about typescript so I tested it to | confirm that it worked. But in Python3, the default arguments are | evaluated at module time if I am not mistaken. The following will | display roughly the same timestamp from datetime | import datetime import time | print(datetime.utcnow()) def show_ts(d=datetime.utcnow()): | print(d) time.sleep(5) show_ts() | | where as in TS, the 2nd one will be roughly 5s older: | const now = Date.now(); console.log(now); | function logTs(d=Date.now()) { console.log(d); } | setTimeout(()=> { logTs(); }, 5000); | horsawlarway wrote: | That's because default params in JS are essentially syntactic | sugar ex - function example(a = 1) { ... | } | | behaves the same as function example(a) { | a = (typeof a !== 'undefined') ? a : 1 ... } | sorahn wrote: | Which also means the suggestion to use ?? has a different | result then using a default param. ?? will catch both null | and undefined, where as a default parameter will only trigger | on undefined. | tempodox wrote: | Events that happen at a later time are called "younger", not | "older". If you were born at a later date than me, you're | younger than me. | nepeckman wrote: | I think #6 (optional properties) is a pretty contrived and | unrealistic example. Most people use optional properties for | extra options or enhancements, not as substitution for an object | hierarchy. | jgwil2 wrote: | I've definitely used them in the way the article describes. | Think about an API that gives a list view and then a detail of | an object that has more properties than are sent in the list | view (maybe because they are expensive to compute or maybe just | to save bandwidth). This scenario can definitely be more | accurately modeled with interface inheritance than with | optional properties. | nepeckman wrote: | Okay your answer is more illustrative to me of the problem | and solution. Totally agree, if you've got a list view and | detail view those should be separate classes. Though my | personal take in that senario is you might be better off | using TypeScripts mapped types to define a subset of your | detail view as a list view. | drew-y wrote: | Agreed. The author is conflating discriminated unions with | optional properties. His example _is_ better off as a | discriminated union because the optionals are set based off of | it 's type. There are plenty of situations where optional | properties do make sense. Particularly where a default value | can be used. | drinchev wrote: | Btw in this context I would like to share Type Fast by | sindresorhus [1]. | | I find it excellent and definitely a helpful tool to think about | when you are struggling with type safety vs runtime data. | | 1: https://github.com/sindresorhus/type-fest | paxys wrote: | Soft disagree with 3. unknown might be better than any but then | the typechecker isn't going to let you read any fields from the | object, even if you are using a null check. And the given example | is weird because you don't need to add a type annotation there at | all. The problem really is that TypeScript doesn't have an | "unknown deserialized JSON" type. | | For #7, in a lot of (most?) cases, the name of a generic isn't | meant to be descriptive. I'd argue that functions that expect it | to be are doing it wrong. If I have a `function | reverseArray<T>(arr: T[])`, the point of T is that it can be | anything. Saying `reverseArray<ArrayTypeToBeReversed>` serves no | purpose. | | TypeScript also really, really needs to have a strict | typechecking mode. Yes I know the "it compiles to raw JS" | arguments, but it could easily add extra checks in the JS code | (basically what example #4 shows). | deepfriedrice wrote: | Good stuff. | | I don't get the example for #3. I understand using "unknown" | instead of "any", but both examples look effectively the same in | terms of type safety. | | I also don't get the example for #4. Side effects are the inherit | "blind spot" of TypeScript. If you're really worried about your | API changing on you, it seems like you'd be better off modeling | it in something like JSON schema. Unless maybe your API is | trivial. | [deleted] | jgwil2 wrote: | In the fixed version of #3, the function return type is | `Product[]` instead of `any`. The example is further refined in | #4. | | What type guards are good for is marrying the compile time | checks with runtime safety. They allow TSC to infer that in a | given code path, the type can be narrowed down to something | more specific. This is nice because it lets a single function | e.g. handle a few different types without littering your code | with `as` or `!`. | ivan888 wrote: | I also don't understand 4. This looks terrible. Why use | TypeScript if we have to make very manual assertions about the | structure? Isn't that the purpose of TS itself? | jakelazaroff wrote: | TypeScript's type checking is static, so on its own it can't | know that values it receives from elsewhere (like JSON coming | from a string) is the correct type. The only option is for | the programmer to check at runtime, which allows TypeScript | to know the type along the code path in which the check | succeeds. | ivan888 wrote: | This makes sense; but it still doesn't feel like the right | answer. Facilities should exist for runtime type checking | based on TS definitions, and syntactic features should | enable a 'hard check' of the actual object when necessary | jakelazaroff wrote: | That runs counter to one of the goals of TypeScript, | which is to make no runtime changes. | | It's easy enough to do this in userland. I wrote a tiny | library called narrows which has worked great for me: | https://www.npmjs.com/package/narrows | ivan888 wrote: | Cool library, the type guards look interesting. Too bad | it's not more automatic but yeah it's not very much extra | code | curtisf wrote: | There are libraries that do type-safe schema validation | (an article I found in a quick Google is linked below). | | That you can do something so sophisticated is a testament | to TypeScript's type system. | | However, if you were writing JavaScript, I think it's | (unfortunately) uncommon to have this kind of proactive | validation. So, for people writing TypeScript as | "JavaScript + types" (which is a perfectly fine way to | use it), they would just use a type assertion, which is | just writing down the assumption that the JavaScript | programmer makes when consuming but not validating | external data. | | https://2ality.com/2020/06/validating-data- | typescript.html#p... | [deleted] | Garlef wrote: | Regarding "don't use `x as Y` but instead use type guards". | | Type guards have a runtime overhead, don't they? | | (You could argue: "If TS can not infer the type at this point so | neither can you. Better to check at runtime." But from my | experience, sometimes TS is simply not smart enough.) | throwanem wrote: | Type guards do have a runtime overhead (since they're functions | that get called), but so does any other method of validating | untyped data at runtime. That's probably the 99% case with type | guards, so I feel like the specific overhead on that isn't such | a huge deal since anything else (e.g. runtypes) would probably | have more. | | (I can think offhand of one case in the last year where I've | wanted to use a type guard for a reason that _didn 't_ have to | do with runtime validation, anyway. Even then, it wasn't the | solution I ended up going with.) | Garlef wrote: | Out of interest: What was the context where you needed the | validation? | | In most contexts, the reasonable approach would be to only do | runtime validation at your system boundaries and not anywhere | inbetween. | throwanem wrote: | The case I mentioned didn't have to do with validation - in | a case like that, I _would_ have used a type guard, or more | likely a runtype. This was a Javascript corner case that I | hadn 't previously encountered, where if you load the same | module from two different paths, they aren't | `instanceof`-equivalent. In this case, both the codebase of | a module I was working on, and the codebase of one of its | consumers, imported the same module from the same package, | and I ran into the issue when I `npm link`-ed the module | into its consumer's node_modules dir for testing and to | ensure I hadn't inadvertently broken an implicit contract. | The `instanceof` mismatch, between an object instantiated | in module code and the class imported by the consumer, | confused the type system and gave me spurious errors as a | result. | | Thinking about how to solve this, it occurred to me to use | a type guard, since it would have effectively convinced the | type checker that my object instance was in fact an | instance of the class of which it is an instance. But it | would've been a wart and also imposed the aforementioned | runtime overhead - not that that would've been meaningful | in this case, it wasn't in a hot loop or anything, but I | still didn't like it. So I ended up instead exporting the | shared dependency from the module and having the consumer | import it from there, rather than from its own (IIRC | otherwise unused and thus eliminated) copy of the shared | dependency. | adamredwoods wrote: | Don't we mostly see this from json responses? The example they | gave adds overhead, checking each product form an array. We use | some responses with 200 products, we don't want this extra | latency. | | Does JS need a better way to introduce json type safety? C# and | graphql use schemas, I wonder if this is a better approach | (compile time, not run time). | Garlef wrote: | If you're receiving JSON data, you can not validate the data | at compile time. | | But yes: I think TS is missing a way to derive runtime | functionality from the type definitions - at least in the | cases, where the types describe plain JSON. | | Something along the lines of clojures' `spec`. | jakelazaroff wrote: | You can do the converse: create a type guard asserting a | structure, and then extract a type. type | Spec<T> = T extends (x: unknown) => x is infer U ? U : | never; type Foo = Spec<typeof someTypeGuard>; | [deleted] | recursive wrote: | Disagree on #10. The amount of code I've seen that intentionally | distinguishes between null and undefined is... well, zero. On the | other hand, the amount of code I've seen that unintentionally | distinguishes between them when it shouldn't have is not zero. | izolate wrote: | I like the "why we do it" part in every section. Kudos to the | author for that. | | However, the type guard section seems a bit off to me. Is the | suggestion to check each and every property of `Product` in | `isProduct`? Seems a bit verbose. | | I tend to use Axios, which uses generics to set the payload | return type (obviously, some trust in your API responses is | needed here): axios.get<Product[]>(...); | sakarisson wrote: | I think the idea is that you validate the data once, on an API | level. You don't need to validate the same data again after the | initial inspection. | | In our team we use io-ts to validate all of our endpoints, but | simple type guards could achieve the same goal. | | https://github.com/gcanti/io-ts | adriancooney wrote: | Runtypes is another _excellent_ alternative. Highly | recommend. | | https://github.com/pelotom/runtypes | horsawlarway wrote: | To me - A type guard lets me be as picky as I'd like to be in | the given circumstance. | | I also use axios, and I also take advantage of the generics | you've shown above, but I acknowledge that I'm essentially just | saying "Trust me" and doing a cast when I do that. | | And I'll add - that exact style of code has been a source of | bugs in our production codebase before. A dev will pick the | wrong type for the axios generic, and if the api response | happens to overlap on the used fields, no one notices. Then it | blows up 6 months later when someone tries to access a field on | the defined type that wasn't actually returned in the api | response. | agloeregrets wrote: | The word in the back of my head wasn't Verbose. It was | `Expensive`. Explicit looping and checking values like that | seems like the answer to 'why is our app slow'. | horsawlarway wrote: | I mostly disagree with you here. | | Checking for the existence of a key on an object is dirt | cheap. | | Even if you happen to have a case where it is expensive, | there's absolutely nothing that says your typeguard has to | take that approach. | | I've seen some reasonably sane code that just checks that the | 'Type' field of the object matches the expected value, and | the 'Version' field is the right number. It's not going to | catch all the possible errors there if someone breaks the api | contract, but it's a lot better than a raw cast. | bwestergard wrote: | The latency will be linear with the size of the JSON payload, | and the constants will be tiny. | llimos wrote: | 11. Going down the rabbit hole spending three whole days getting | your typings perfect for some weird use case, instead of writing | actual code. _Sometimes_ there 's a benefit down the line to | having done it, often not, and knowing the difference is where | greatness lies. | Madeindjs wrote: | I completely agree with you. I once use JSdoc instead of | Typescript for a small project. Typing works as good as | Typescript on VS Code. | keyle wrote: | This! | | I found it far more productive that going through rabbit hole | of TS and its compiler. I write my `definitions.d.ts` for my | objects, and use them simply /** @type | {ns.MyType} identifier */ const identifier... | | and voila, my IDE gives me auto-completes and warns me when | I'm doing silly stuff. | | It works for functions as well using /** | @param {type} parameter name */ | | and is generally a good idea to use. | | It gets you 80% of the way there with no compiler/transpiler | (although one might argue that the IDE is compiling non-stop | on file saves, with its language server). | | I embraced Typescript when it was new and the Javascript | standard was a total mess. It was leaps and bounds ahead and | closer to something like AS3. At the time TS was godsend. | | Today, JS + webpack gets you really far using the latest | ECMAscript standards. | | Same with IDE. The amount of stuff my IDE can deduct based on | my few typings and my use of modern strict javascript has | strongly reduced the need for another transpiler (on top of | webpack). | | I found jsDoc to be a perfect 80-20 solution. | | Because at the end of the day, TS isn't a compiler, it's a | transpiler. You're not compiling to bytecode in a vm, you're | still in javascript. | | This saves my bacon in the front-end. For the back-end, I | most definitely choose a strongly typed language like Go. | jamil7 wrote: | I kind of came to the same conclusion at some point. I normally | find statically typed languages much more productive. If you're | working with all typescript libraries with great typings it's | probably great but the reality of the javascript ecosystem | means you're often wasting time getting the compiler to | understand how a few dependencies should cooperate. That and | the fact that I contracted of a few larger projects that were | in an transition phase filled with 'any' which often meant | little type safety but a lot of extra typing. | Garlef wrote: | This is definitely something that typescript constantly lures | you into. | | I recently switched back to JS for a prototype of a programming | language. | | I immediately felt more productive. | | At times, the TS systems feels like a theorem prover. Until it | doesn't. And then you're left with a bunch of types that | _almost_ do what you want... but not quite; Always tempted to | experiment again for a day or two. | | But let's see what happens down the road. Maybe I'll miss the | incidental documentation the typings provide. | acemarke wrote: | Absolutely. One of my strongest opinions about TS is that you | should use it pragmatically. It's not worth burning hours of | time trying to placate the TS compiler if you know the code | works okay - throwing in a `type $FixTypeLater = any` and | moving on is an acceptable workaround depending on the | situation: | | https://blog.isquaredsoftware.com/2019/11/blogged-answers-le... | MattGaiser wrote: | I have been working on a frontend project for the past two | weeks now. Probably about half my time has been dealing with | type errors. | calebegg wrote: | I think calling your generic type 'Element' is a bad choice -- | that's shadowing a built-in type | (https://developer.mozilla.org/en-US/docs/Web/API/Element) and it | makes the code confusing to read at a glance. | | That's one thing I like about T, it's very self evident that it's | just 'the' generic type. But maybe I'm just used to it. With | multiple generic types I can see how longer names would be | useful. | hooksfordays wrote: | Agreed. I think naming generic types makes the most sense with | multiple types, or when more specific names make sense. For | example, if a generic argument must inherit from a certain | type: `class SomeView<ViewState extends BaseViewState>` or some | such. | | `Element` is as non-descriptive as `T` in the author's example | and, IMO, doesn't add any tangible benefit. | elnygren wrote: | Good points! | | Seems like for "6. Optional properties" | | One should rather use tagged unions (see: Abstract Data Types, | variants) that are usually written like this in TS: | type Product = | { type: 'digital', id: string, | sizeInMb: number } | { type: 'physical', id: string, | weightInKg: number } | | At least I find it more elegant, concise and fun to work with :) | bengalister wrote: | According to | https://github.com/microsoft/TypeScript/wiki/Performance#pre... | using union types in your example vs interfaces like #6 is bad | for compilation performance. | agloeregrets wrote: | I like this way more. Wayyy more. | elnygren wrote: | Also, for using type guards to validate incoming data, check | out https://github.com/pelotom/runtypes. | | Or if brave enough to dive into the deep end of FP, io-ts is | nice. https://github.com/gcanti/io-ts | dbartholomae wrote: | Hi everyone! Happy to see my article shared here. If there are | any questions, just let me know. And yes, the list is opinionated | ;) Especially the question on one-letter generic types has been | debated a lot. | drenvuk wrote: | I think using it would be the first one. | | I don't think I'll ever be able to convert to using typescript, | I'd rather have validators running in the code checking external | inputs for the proper fields at runtime before the rest of the | system blithely processes whatever it gets due to safe | assumptions. Add integration test for specific modules and | everything works fine. | | I don't want to compile languages that don't need compilation and | I don't like the fact that I need to create all of these fake | feeling types which are actually just objects rather than | javascript classes. It's just feels like boilerplate when you | could instead include the 'type' that it's supposed to be in the | name and look up a validator for that 'type'. | | I'm probably wrong, but it really doesn't feel like it. | koolba wrote: | Static typing check is a huge boost for code refactoring. It | allows you to quickly make changes and be reasonably confident | that you haven't horrendously broken something. | | It doesn't obviate the need for dynamic validation nor is it | meant to do that. But it does give you a clear point of where | to perform those validations rather than littering it | everywhere throughout your application. | | It doesn't obviate the need for testing either. But it does | eliminate an entire class of dumb programming errors such as | incoming a function with the wrong arguments. | throwanem wrote: | Yeah, I mean I don't want to just say "you're wrong", but you | kind of are here. | | TypeScript alone doesn't make any assertions and doesn't offer | any checking at system boundaries ("external inputs"). That's | not what it is for, and the types you specify with it aren't | objects. All that stuff gets compiled out before the code is | even run. The point of TypeScript is so that you don't _have_ | to run your code to know whether the way it handles data | internally is the way you intend it to. | | Validation at system boundaries is a separate concern, and | handled separately. I like to do it with a package called | "runtypes", which both supports arbitrarily complex shapes in | its validators, and expresses them in a way that's very close | to TS and can be trivially converted into compile-time TS types | so that you don't have to write the same types twice. | | I wasn't sure what to make of TypeScript for a while, too. But | having once tried it, and since become very conversant with it, | I'll go back only for trivial tasks or under significant | duress. Sure, writing types is a fair bit more work up front | and a little more ongoing, but it really does make a huge | difference in maintainability; in my experience, it's a little | work now to save a _lot_ of work later, as the resulting code | is both less likely to exhibit an entire class of often quite | subtle bugs, and easier to reason about and modify. | h1fra wrote: | I think the code sample after `Use the new ?? operator, or, even | better, define the fallback right at the parameter level.` is not | correct, it stops after new Date without any mention of ?? | | edit: read too fast :facepalm: | dbartholomae wrote: | It uses a fallback at parameter level instead of the ?? | operator. | h1fra wrote: | oops indeed, but I'm not crazy there was a missing | parathensis no? It's too late :D | | Anyway great blog post | dbartholomae wrote: | Yes, someone found the typo, send it to me via Twitter, and | I fixed it. This is also where I know from that the article | is on Hacker News :D | ulucs wrote: | It's defined at the parameter level, using function parameter | defaults | jgwil2 wrote: | Yeah, the example just shows the "even better" option: it gives | a default value for the date parameter. | [deleted] | agloeregrets wrote: | As much as I agree with most of this, I also think that some of | it can lead to some weird issues for new devs or dangerously, | lead to a world where people are helpless once they accidentally | break out of the world of typescript. In a few cases, there is | actual runtime overhead for the choices made, for example the | typechecking example. | dlbucci wrote: | I agree with most of these, but I pretty strongly disagree with | #10 (don't use `!= null`). The reasoning is that you can use | `null` and `undefined` to mean different things, like `null` | being "no first name" and `undefined` meaning "haven't asked | yet". I agree that strict TS lets you do stuff like that fairly | easily, but I think this is a really bad idea, because there's | nothing that would indicate to a reader of that code the meaning | of `null` vs `undefined` for those properties. | | I feel in that case you should have some type union of | `"notAsked" | "none" | { t: "some", v: "Name" }`, because that | would be more clear of what each possible value means. Let `null` | and `undefined` be synonyms for `nil`. | codefined wrote: | I've been regularly trying to reduce / remove usage of `null` | in various projects. Sindresorhus has some interesting | discussions on the topic here[0]. | | [0] https://github.com/sindresorhus/meta/discussions/7 ___________________________________________________________________ (page generated 2021-02-02 23:00 UTC)