Add undefined to JSON.stringify return type #51897
Conversation
This reverts commit 23f641c.
|
@ronyhe please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
Even though this is kind of better than it was before, there's still a variety of cases when this typing fails. JSON.stringify({toJSON: () => undefined}) |
Do you have a suggestion as to how to approach this? |
|
@microsoft-github-policy-service agree company="Wix.com" |
|
@ronyhe the command you issued was incorrect. Please try again. Examples are: and |
Once I wrote down "correct" types for JSON methods, and those were utterly dreadful. Correct approach is to reexport them somewhere in your project with types tightened as much as possible (without any undefined, toJSON, replacers and overloads allowed), and avoid spending any time to handle accidental complexity of EcmaScript standard and TS type system. It doesn't pay off. |
Then let's leave the |
|
I'm considering extracting out the replacer type: type JsonReplacer = number[] | string[] | ((this: any, key: string, value: any) => any)
interface JSON {
/**
* Converts a JavaScript Object Notation (JSON) string into an object.
* @param text A valid JSON string.
* @param reviver A function that transforms the results. This function is called for each member of the object.
* If a member contains nested objects, the nested objects are transformed before the parent object is.
*/
parse(text: string, reviver?: (this: any, key: string, value: any) => any): any;
/**
* Converts a JavaScript value to a JavaScript Object Notation (JSON) string.
* @param value A JavaScript value, usually an object or array, to be converted.
* @param replacer An array of strings and numbers that acts as an approved list for selecting the object properties that will be stringified.
* @param space Adds indentation, white space, and line break characters to the return-value JSON text to make it easier to read.
*/
stringify(value: Function | Symbol | undefined, replacer?: JsonReplacer | null, space?: string | number): undefined;
/**
* Converts a JavaScript value to a JavaScript Object Notation (JSON) string.
* @param value A JavaScript value, usually an object or array, to be converted.
* @param replacer A function that transforms the results.
* @param space Adds indentation, white space, and line break characters to the return-value JSON text to make it easier to read.
*/
stringify(value: any, replacer?: JsonReplacer | null, space?: string | number): string;
}Any thoughts? |
|
@ronyhe What for? It's only used twice, and moving it to a dedicated type is yet another potential break (if someone has such a type defined). Also, the TypeScript team generally doesn't want to add more utility types:
|
@MartinJohns The original replacer type has two options (not including
|
I don't understand this. Extracting it to a utility type it's still a union. And overloads are not interchangeably with unions, they behave very much different. |
|
@MartinJohns Yes, I suppose you're right. |
Add undefined to JSON.stringify return type when input value is Function | Symbol | undefined
Fixes #18879