json-schema-to-typescript
json-schema-to-typescript copied to clipboard
bcherny
additionalProperties can create invalid typings
The following schema:
{
"properties": {
"s": { "type": "string"}
},
"additionalProperties": {
"properties": {
"foo": {"type" : "string"}
}
}
produces this typing:
/* tslint:disable */
/**
* This file was automatically generated by json-schema-to-typescript.
* DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file,
* and run json-schema-to-typescript to regenerate this file.
*/
export interface FooSchema {
s?: string;
[k: string]: {
foo?: string;
[k: string]: unknown;
};
}
Which produces this tsc error:
foo.d.ts:9:3 - error TS2411: Property 's' of type 'string' is not assignable to string index type '{ [k: string]: unknown; foo?: string; }'.
Hey there! In this case, it's not clear to me what the expected behavior is here. It really is a contradiction that s must be both a string, and an object with a key foo.
Instead, consider expressing this using "oneOf", to make it clear that you want a shape that's one or the other, and not both. Something like:
{
"oneOf":[
{
"properties":{
"s":{
"type":"string"
}
},
"additionalProperties":false
},
{
"additionalProperties":{
"properties":{
"foo":{
"type":"string"
}
},
"additionalProperties":false
}
}
]
}
Which generates:
export type Demo =
| {
s?: string;
}
| {
[k: string]: {
foo?: string;
};
};
The schema is not unclear though? s is always a string and any other properties are objects with a foo key.
This is a fine type for what I want, which is exactly what my schema means:
/* tslint:disable */
/**
* This file was automatically generated by json-schema-to-typescript.
* DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file,
* and run json-schema-to-typescript to regenerate this file.
*/
export type FooSchema = {
s?: string;
} & {
[k: string]: {
foo?: string;
[k: string]: unknown;
};
}
Maybe JSONSchema and TS have different semantics here:
export type S = {
s?: string;
} & {
[k: string]: {
foo?: string;
[k: string]: unknown;
};
}
let s: S = {s: 'x'}
/*
Error TS2322:
Type '{ s: string; }' is not assignable to type 'S'.
Type '{ s: string; }' is not assignable to type '{ [k: string]: { [k: string]: unknown; foo?: string | undefined; }; }'.
Property 's' is incompatible with index signature.
Type 'string' is not assignable to type '{ [k: string]: unknown; foo?: string | undefined; }'.
*/
What you're trying to express is: "the field s has shape X, and every field that is not s has shape Y". The problem you're running into is you can't express "every field that is not s" in TS's type system -- you can express "every string field", which includes s.
My typing work for my case which is handling input from non ts code (see this) and I need to make sure that the ts code treats it properly.
It is true there is a bit of an issue with typescript and this sort of thing, but there is certainly a better solution that what it compiles to now.
I feel this is definitely a bug. My schema is valid and unambiguous. This should either be documented as a shortcoming or it should compile to something at least valid in ts. If you wanted it to compile to your union thing that would be better than producing an error at least.
I agree. The current behavior of producing TypeScript files that fail to compile is broken. It either should fail during this tools runtime or produce a type that captures as much of the schema meaning as possible.
While I’m not sure it’s technically feasible to guarantee that the result typechecks, maybe we can start by doing better in this case in particular.
Here’s the TS issue that blocks this one: https://github.com/microsoft/TypeScript/issues/17867. I’d love ideas for a workaround if someone has one, but I’m not sure there’s a great way to do this today.
Similar issue: generated types are too common.
For example, generated type is
type A = {
x: number;
[k: string]: unknown;
}
Type of x does not matter here. TS will use unkown and allow to set anything in x:
const test: Omit<A, "y"> = {
x: "bla",
}
https://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=10&pc=7#code/C4TwDgpgBAglC8UDeAoK6oA8BcUB2ArgLYBGEATgNxoYDaA1rgM7DkCWeA5gLq4F708AewDueagF8UKAMZC8LKMAgtcAeSJtgAHhgAaKACIQhgHwJkNdDiMkANgENDelFJTKWAOkxA
It would be handy to have an option that ignores additionalProperties: true. TS allows unspecified properties in runtime anyway.
Hi,
I see a long discussion also in the pr. In my case, I have a schema that generates something as:
{
"."?: PackageExportsEntry | PackageExportsFallback;
[k: string]: PackageExportsEntry | PackageExportsFallback;
}
Whilst imho the simple correct rendering should be:
{
"."?: PackageExportsEntry | PackageExportsFallback;
[k: string]: PackageExportsEntry | PackageExportsFallback | undefined;
}
The error is due to the fact that the index type is not optional, so making it in | undefined fix the issue.
The JSON schema I used is the official schema for package json.
Not sure if this would fix all the cases mentioned in this issue and in the pr, nor if my proposed fix is actually correct.
But it could make sense, since accessing a field whose name is not known, it could truly create a undefined value, so at least it is safe to mark it as possibly undefined.
Regards
I raised #477 to cover that case @FStefanni but it was closed as a duplicate of this issue. Commenting here as well just in case comments on closed issues aren't monitored.
Personally I don't see why this issue duplicates the one that we're having with the package.json schema. That is, I literally just want strictIndexSignatures to do exactly what the docs say that it does and "[a]ppend all index signatures with | undefined".
The fact that it doesn't may be related to this issue, and it may resume working as documented when this issue is resolved, but if the intended behaviour of the strictIndexSignatures flag is really as simple as what's in the doc, it doesn't really make sense to me to block resolving that very narrowly-scoped problem on a resolution for this wider problem.
