Change json/jsonb type
abenhamdine opened this issue ยท 6 comments
Currently, a json or jsonb db field is mapped with typescript Object
type.
Though Object would seems appropriate at first sight, it leads to following type errors :
Property 'foo' does not exist on type 'Object'.
...sinceObject
type (same with object
and {}
ts types) doesn't accept more properties.
It should be more appropriate to type these fields with a specific type which accepts any properties, something like :
interface JsonParsed extends Object {
[k: string]: any
}
or more precise :
interface JsonParsed {
[k: string]: string|number|boolean|Date|JsonParsed|JsonParsedArray|null;
}
interface JsonParsedArray extends Array<string|number|boolean|Date|JsonParsed|JsonParsedArray|null> { }
What do you think about it ?
Looks good!
In our use-case, we have specific interface for each jsonb object, and we extend the generated schemats table interfaces with our own. i.e.
interface SomeJsonbType {
foo: string
bar: number
}
import { ATable } from './dbname.schemats'
export interface OurTable extends ATable {
somejsonb: SomeJsonbType
}
I think most users wouldn't probably want to go through the same level of verbosity, so having a more generic jsonb type is appropriate.
My only concern with choice number 2 is about whether each driver applies their own parsing method to json and jsonb -- i.e. Date is not a part of json value but drivers may decide to parse ISO datestring into Date objects.
I'm leaning more towards
interface JsonParsed extends Object {
[k: string]: any
}
My only concern with choice number 2 is about whether each driver applies their own parsing method to json and jsonb -- i.e. Date is not a part of json value but drivers may decide to parse ISO datestring into Date objects.
Good remark.
Though the more I think about it, the more the 2nd choice looks better, because any
actually disable typechecking, so nothing would prevent to do something like :
interface Json extends Object {
[k: string]: any
}
const json: Json = { foo: 'bar' }
json.myMethodWhichDoesntExist('foo') // NO TS ERROR
while with 2d implementation :
interface JsonParsed {
[k: string]: string | number | boolean | Date | JsonParsed | JsonParsedArray;
}
interface JsonParsedArray extends Array<string | number | boolean | Date | JsonParsed | JsonParsedArray> { }
const jsonParsed: JsonParsed = { foo: 'bar' }
jsonParsed.myMethodWhichDoesntExist('foo') // TS ERROR
You're right, json parsing result may vary depending on the driver (and for instance, for node-postgres, there's still a debate on how dates should be parsed as Date object or no, see brianc/node-pg-types#50).
Besides, some users may choose to override the types parsed by the driver in their application code (as you know, node-postgres allow this, I don't know for other drivers).
I think it's an acceptable tradeoff. And in a second time, it will be still possible to implement an option to generate the JsonParsed type with or without the Date
type in the union.
Sounds good, let's go with
interface JsonParsed {
[k: string]: string | number | boolean | Date | JsonParsed | JsonParsedArray;
}
interface JsonParsedArray extends Array<string | number | boolean | Date | JsonParsed | JsonParsedArray> { }
const jsonParsed: JsonParsed = { foo: 'bar' }
jsonParsed.myMethodWhichDoesntExist('foo') // TS ERROR
Should be noted, though, that postgres allows any json value to be typed as json, and does not limit it to objects or arrays:
> SELECT '"foo"'::json;
json
----
"foo"
Really JSON and JSONB should be typed as unknown
because you should really be running them through type guards before doing anything with the contents.