Skip to content

narrow AnyType to StringType in mapped types #2412

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.

Sign up for GitHub

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

Merged
arthurfiorette merged 8 commits into from
Dec 1, 2025
Merged

narrow AnyType to StringType in mapped types #2412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a73dac3
get basic case
srsudar Nov 25, 2025
e5fea9f
fix test
srsudar Nov 25, 2025
31bec0e
add some tests
srsudar Nov 26, 2025
098b4e7
better file
srsudar Nov 26, 2025
79548ac
include test for template literal
srsudar Nov 26, 2025
5ac6aaf
rm thing
srsudar Nov 26, 2025
ccab3b1
correct template literal
srsudar Nov 26, 2025
0e8140f
Merge branch 'next' into sam-any-index-type
srsudar Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/NodeParser/MappedTypeNodeParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { NodeParser } from "../NodeParser.js";
import { Context } from "../NodeParser.js";
import type { SubNodeParser } from "../SubNodeParser.js";
import { AnnotatedType } from "../Type/AnnotatedType.js";
import { AnyType } from "../Type/AnyType.js";
import { ArrayType } from "../Type/ArrayType.js";
import type { BaseType } from "../Type/BaseType.js";
import { DefinitionType } from "../Type/DefinitionType.js";
Expand Down Expand Up @@ -55,7 +56,8 @@ export class MappedTypeNodeParser implements SubNodeParser {
if (
keyListType instanceof StringType ||
keyListType instanceof NumberType ||
keyListType instanceof SymbolType
keyListType instanceof SymbolType ||
keyListType instanceof AnyType
Copy link
Copy Markdown
Collaborator

@arthurfiorette arthurfiorette Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do the same for unknown? what are all valid ts.types to index an object?

Copy link
Copy Markdown
Contributor Author

@srsudar srsudar Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point. With this code:

type Foo = {
  [key: boolean]: unknown;
};

This is the error I get:

An index signature parameter type must be 'string', 'number', 'symbol', or a template literal type.

But afaik, everything gets coerced to a string anyways in TS. Eg:

$ node
Welcome to Node.js v20.18.0.
Type ".help" for more information.
> t = {};
{}
> t[1] = 'the number one';
'the number one'
> t[1]
'the number one'
> t['1']
'the number one'
> t
{ '1': 'the number one' }
> t['1'] = 'the string one'
'the string one'
> t
{ '1': 'the string one' }
> t[undefined] = 'this was undefined'
'this was undefined'
> t[undefined]
'this was undefined'
> t['undefined']
'this was undefined'

So I would think that this perhaps could just never throw, and instead always default to string? As-is, this it kind of seems like an additional, more restrictive typecheck.

Copy link
Copy Markdown
Member

@domoritz domoritz Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't keys be symbols (in JS and TS) or string literals (in TS) as well? So it's not always string, right?

Copy link
Copy Markdown
Contributor Author

@srsudar srsudar Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the full list according to the error message:

An index signature parameter type must be 'string', 'number', 'symbol', or a template literal type.

But I thought pretty much anything can be a key due to toString behavior. eg:

$ node
Welcome to Node.js v20.18.0.
Type ".help" for more information.
> t = {};
{}
> obj = { hello: 'there'};
{ hello: 'there' }
> t[obj] = 'object value';
'object value'
> t
{ '[object Object]': 'object value' }
> obj.toString()
'[object Object]'

It looks like Symbol does make it into the object, but it doesn't survive JSON.stringify(). Wow, TIL.

I'm not sure what makes the most sense here. I'm pretty new to working with the TS AST like this. Happy to try whatever you think makes sense.

Copy link
Copy Markdown
Member

@domoritz domoritz Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you try Symbol?

Copy link
Copy Markdown
Member

@domoritz domoritz Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think start by making more tests cases with more types (ideally in the same test example rather than creating a ton of examples).

Copy link
Copy Markdown
Contributor Author

@srsudar srsudar Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symbol here:

> s = Symbol('symb');
Symbol(symb)
> s
Symbol(symb)
> t[s] = 'symbol value';
'symbol value'
> t
{ '[object Object]': 'object value', [Symbol(symb)]: 'symbol value' }
> JSON.stringify(t)
'{"[object Object]":"object value"}'
>

By "ideally in the same test example rather than creating a ton of examples" do you mean just in a single main.ts and schema.json file?

Copy link
Copy Markdown
Member

@domoritz domoritz Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Copy Markdown
Contributor Author

@srsudar srsudar Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that you guys had already covered most of these, and in a pattern that I was ignoring.

number is handled here:

https://github.com/vega/ts-json-schema-generator/blob/next/test/valid-data/type-mapped-number/main.ts

symbol is handled here:

https://github.com/vega/ts-json-schema-generator/blob/next/test/valid-data/type-mapped-symbol/main.ts

I copied this for any and for a template literal (which I think are the last two that are uncovered, based on this:

An index signature parameter type must be 'string', 'number', 'symbol', or a template literal type.

I stuck both of these in their own files, to keep the pattern going. Lmk if you'd prefer them in a single file. I confirmed that Record<any, string> is failing without this change.

I feel like this shouldn't need to handle unknown, since that isn't valid TS I don't think.

) {
if (constraintType?.getId() === "number") {
const type = this.childNodeParser.createType(
Expand Down
4 changes: 4 additions & 0 deletions test/valid-data/type-mapped-any/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { assertValidSchema } from "../../utils";
import { test } from "node:test";

test("type-mapped-any", assertValidSchema("type-mapped-any", "MyObject"));
1 change: 1 addition & 0 deletions test/valid-data/type-mapped-any/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type MyObject = Record<any, string>;
12 changes: 12 additions & 0 deletions test/valid-data/type-mapped-any/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"$ref": "#/definitions/MyObject",
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"MyObject": {
"additionalProperties": {
"type": "string"
},
"type": "object"
}
}
}
4 changes: 4 additions & 0 deletions test/valid-data/type-mapped-template-literal/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { assertValidSchema } from "../../utils";
import { test } from "node:test";

test("type-mapped-template-literal", assertValidSchema("type-mapped-template-literal", "MyObject"));
3 changes: 3 additions & 0 deletions test/valid-data/type-mapped-template-literal/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type AorB = "A" | "B";

export type MyObject = Record<`letter-${AorB}`, string>;
22 changes: 22 additions & 0 deletions test/valid-data/type-mapped-template-literal/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"$ref": "#/definitions/MyObject",
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"MyObject": {
"additionalProperties": false,
"properties": {
"letter-A": {
"type": "string"
},
"letter-B": {
"type": "string"
}
},
"required": [
"letter-A",
"letter-B"
],
"type": "object"
}
}
}
Loading