ts-reset icon indicating copy to clipboard operation
ts-reset copied to clipboard

Add strong types for JSON.stringify

Open sstur opened this issue 2 years ago • 7 comments

The built-in types for JSON.stringify() are not correct/sound because JSON.stringify() will return undefined if undefined is passed in (or function or symbol). Many devs don't know this and TS types don't reflect this possibility, resulting in a false sense of null safety and often runtime exceptions in prod.

This fixes that using conditional types. If there's a better way to express this other than conditional types please let me know and I'll update this PR to reflect it.

Thanks!

sstur avatar Feb 20 '23 22:02 sstur

  • Typescripts issue for this is here https://github.com/microsoft/TypeScript/issues/18879

KotlinIsland avatar Mar 22 '23 03:03 KotlinIsland

Thanks @KotlinIsland! As you probably discovered, a correct typedef for JSON.stringify was attempted and even landed in TS core, but it was rolled back because it broke too much real-world code. According to RyanCavanaugh in his comment here this is not likely to make it into core.

I suppose stuff like this is a big part of why ts-reset exists!

sstur avatar Mar 22 '23 04:03 sstur

@KotlinIsland Added support for return type specialization in my PR, #124. It also correctly handles toJSON, constructor functions, and symbols.

@sstur I also copied your test cases into my PR and all of them pass. In addition, I added several more test cases for the replacer function. If you're happy with the changes that I made, we could close this PR in favor of mine.

aaditmshah avatar Mar 27 '23 12:03 aaditmshah

Allow me to add a vote of support for this. I have recently experienced a bug in our codebase because of the incorrectly lax type of JSON.stringify.

For what it’s worth, I recommend against always returning string | undefined. The amount of non-null assertion operators it has added in our codebase when we tried it was too large, and led to basically completely ignoring this strict type. It’s okay to have a few false negatives, but not all of them.

denis-sokolov avatar Sep 13 '23 05:09 denis-sokolov

@denis-sokolov Shameless plug. Consider using better-typescript-lib instead of ts-reset. Version 2.3.0 of better-typescript-lib added strong type definitions for JSON.parse, JSON.stringify, and fetch().json(). It also added better type definitions for promises. I can vouch for these type definitions because I implemented and tested them myself.

I originally implemented these type definitions in ts-reset.

  1. #121
  2. #123
  3. #124

However, my PRs were summarily dismissed by @mattpocock. Matt's a brilliant guy, and he's doing god's work by spreading the gospel of sound typing in TypeScript. But, he's not open to new ideas, which is why I'm doubtful of his open-source leadership skills.

On the other hand, the author of better-typescript-lib, @uhyo, is a genuinely nice guy who listens to others people's ideas. And, the second biggest contributor to better-typescript-lib, @graphemecluster, is a veritable type wizard, just like Matt. I've also seen him answer questions on https://es.discourse.group/. He's a smart guy.

Anyway, I'm not recommending better-typescript-lib to you just because of it's contributors. I'm recommending it to you because I honestly believe that it's superior to ts-reset in every metric.

  1. It doesn't require users to define a reset.d.ts ambient type declaration file. If you're using npm or yarn, then all you need to do is install better-typescript-lib as a dev dependency, and you're golden. If you're using pnpm, as I do, then you will also need to configure pnpm to hoist @typescript/* packages in order to use better-typescript-lib.
  2. ts-reset doesn't actually replace the built-in type definitions. It just adds additional type overloads to functions. Hence, it's still possible to shoot yourself in the foot with ts-reset. On the other hand, better-typescript-lib actually replaces all the built-in type definitions with entirely new type definitions. It's able to do this because of an obscure new feature introduced by TypeScript 4.5. This means that you will only see the new and improved type definitions in your IDE when you use better-typescript-lib. If you use ts-reset, you will still see the old type definitions.

In conclusion, better-typescript-lib is objectively more well-designed than ts-reset. In addition, it provides strong type definitions for JSON.parse, JSON.stringify, and fetch().json() today. And last but not least, it's maintained by an excellent open-source leader, uhyo, who listens to his users and responds to issues and pull requests in a timely manner. Hence, you can expect to receive good support going forward.

aaditmshah avatar Sep 13 '23 09:09 aaditmshah

No offense to him. He's a brilliant guy [...]

Hmm, I wonder if the very next thing will be an offense 🤔

he's not open to new ideas, which is why I'm doubtful of his open-source leadership skills. [Some other guy] is a genuinely nice guy who listens

I guess I should have seen that coming; whenever someone opens with "No offense" it's likely it will be followed by something offensive 🤣

Anyway, better-typescript-lib looks interesting, will check it out.

sstur avatar Sep 13 '23 14:09 sstur

No offense to him. He's a brilliant guy [...]

Hmm, I wonder if the very next thing will be an offense 🤔

he's not open to new ideas, which is why I'm doubtful of his open-source leadership skills. [Some other guy] is a genuinely nice guy who listens

I guess I should have seen that coming; whenever someone opens with "No offense" it's likely it will be followed by something offensive 🤣

Thank you for the writing tip. Fixed. If you don't want to offend people, then never write "no offense". I'll make that a habit. 🙂

Removed "No offense to him."

aaditmshah avatar Sep 13 '23 15:09 aaditmshah