Add partial json type that allows for optional keys#417
Add partial json type that allows for optional keys#417vidartf wants to merge 3 commits intophosphorjs:masterfrom
Conversation
|
Note: While the logic in |
| function isArray(value: ReadonlyJSONValue): boolean { | ||
| function isArray(value: PartialJSONValue): value is PartialJSONArray; | ||
| export | ||
| function isArray(value: ReadonlyPartialJSONValue): value is ReadonlyPartialJSONArray; |
There was a problem hiding this comment.
This can break third party code which is using the functions as type assertion checks, since it's changing the return type. Instead, let's add overloads for each of these functions.
There was a problem hiding this comment.
I think the current code only adds overloads, and leaves the return type untouched. It does change the argument type though.
There was a problem hiding this comment.
Do you have an example that currently works, but will fail after this change? It would make it a lot easier to get this right.
There was a problem hiding this comment.
Note: With the current code, these narrowings are in effect:
// Same as before:
let a = [] as JSONValue;
let b = [] as ReadonlyJSONValue;
if (JSONExt.isArray(a)) {
// a: JSONArray
}
if (JSONExt.isArray(b)) {
// b: ReadonlyJSONArray
}
// New possibilities:
let c = [] as PartialJSONValue;
let d = [] as ReadonlyPartialJSONValue;
if (JSONExt.isArray(c)) {
// c: PartialJSONArray
}
if (JSONExt.isArray(d)) {
// d: ReadonlyPartialJSONArray
}
Refactor of #303 to instead add new partial types that allow undefined value and/or missing keys ("Partial" types in TS jargon).
Some of the type names get a little long, but at least they are explicit.