Skip to content
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

Type signature for JSON.stringify does not include undefined in the return type #18879

Closed
MazeChaZer opened this issue Oct 2, 2017 · 16 comments · Fixed by #51897
Closed

Type signature for JSON.stringify does not include undefined in the return type #18879

MazeChaZer opened this issue Oct 2, 2017 · 16 comments · Fixed by #51897
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Milestone

Comments

@MazeChaZer
Copy link

Although calling JSON.stringify may return undefined (as documented here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Description), it is not included in the type signature provided with TypeScript.

@MazeChaZer
Copy link
Author

This is also true for Edge:

Values that do not have JSON representations, such as undefined, will not be converted.

https://docs.microsoft.com/en-us/scripting/javascript/reference/json-stringify-function-javascript

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this labels Oct 2, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Oct 3, 2017

So we need a new overload for stringify(value: undefined, .....): undefined; ?

@MazeChaZer
Copy link
Author

Thanks for the quick response!

That could work, please note that JSON.stringify(function() {}) also results in undefined.
Perhaps I should explain why this was a problem for me: I wrote an error handling function that would display JSON data

function showError(value: any): string {
    const json = JSON.stringify(value);
    // ...
    // Later in the code, a string function was used
    const lines = json.split('/n');
    // This is where I got the runtime exception
}

Would the overload catch this possible runtime error?

Something that would definitly work for me is a union return type that includes undefined, like so

stringify(value: any /* space, replacer, ... */): string | undefined

But of course I don't know conventions or guidelines for the type annotations very well, so this might be too broad.

@mhegazy mhegazy added this to the Community milestone Oct 4, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Oct 4, 2017

PRs welcomed. You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

@MazeChaZer
Copy link
Author

👍 Will do

MazeChaZer pushed a commit to MazeChaZer/TypeScript that referenced this issue Oct 11, 2017
JSON.stringify() may return undefined if the given input value is undefined or if the input value is not serializable (like `function(){}`).
See microsoft#18879
MazeChaZer pushed a commit to MazeChaZer/TypeScript that referenced this issue Oct 11, 2017
JSON.stringify() may return undefined if the given input value is undefined or if the input value is not serializable (like `function(){}`).
See microsoft#18879
MazeChaZer pushed a commit to MazeChaZer/TypeScript that referenced this issue Dec 5, 2017
JSON.stringify() may return undefined if the given input value is undefined or if the input value is not serializable (like `function(){}`).
See microsoft#18879
@bradleyayers
Copy link

I just discovered a production bug that this would have caught. Any updates on it @MazeChaZer?

@NN---
Copy link

NN--- commented Apr 4, 2018

@mhegazy I suggest having both return value string|undefined and additional overload for undefined|Function|symbol to catch issues at compile time.

@RyanCavanaugh RyanCavanaugh modified the milestones: Community, Backlog Mar 7, 2019
clarfonthey added a commit to clarfonthey/TypeScript that referenced this issue Aug 9, 2022
@greyblake
Copy link

Any progress on this one? (I got my fingers burned with this today)

@reverofevil
Copy link

@RyanCavanaugh Can we triage this? There's no movement on the critical issue for 5 years.

@ronyhe
Copy link
Contributor

ronyhe commented Dec 14, 2022

Here's a PR for this issue: #51897
This is my first attempt at contributing to the TS repo, advice and/or corrections would, of course, be welcome

@ronyhe
Copy link
Contributor

ronyhe commented Dec 19, 2022

Here's a PR for this issue: #51897 This is my first attempt at contributing to the TS repo, advice and/or corrections would, of course, be welcome

Ready for review

@ronyhe
Copy link
Contributor

ronyhe commented Dec 21, 2022

Any updates?
@MartinJohns Is there anything I should be doing to advance this?

@ecyrbe
Copy link

ecyrbe commented Jan 21, 2023

@sandersn can you reopen this bug since the PR got reverted ?

@RyanCavanaugh
Copy link
Member

Having tried this and looked into it more, I don't think we're going to try this again. People can add this line to their project to opt into this behavior if they want it:

interface JSON { stringify(s: any): string | undefined; }

but if we add this today, there is no real way to opt back in to today's behavior.

However, if at some point we make a "stricter" version of the core lib, this would definitely be a change to consider in that feature

@DetachHead
Copy link
Contributor

if at some point we make a "stricter" version of the core lib,

is there an open issue for this?

@KotlinIsland
Copy link

KotlinIsland commented Mar 22, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment