-
Notifications
You must be signed in to change notification settings - Fork 222
add support for a types array #2410
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.
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
Changes from 8 commits
f40b65d
fcc6e38
a5171a1
f8ac2d4
72164e6
7020a1e
a7a8208
8c3ff40
4bf3f07
58dbdf1
11dfab5
a8d159b
7aa46a5
974c8a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,10 @@ export interface Config { | |
|
|
||
| /** | ||
| * Name of the type/interface to generate schema for. | ||
| * Can specify more than once to generate multiple schemas. | ||
| * Use "*" to generate schemas for all exported types. | ||
| */ | ||
| type?: string; | ||
| type?: string | string[]; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not positive this is the right way to type this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks reasonable
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of how you set a
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If so, i think you can simplify the code by removing the castArray and letting all related types be a single array[] (Also remove undefined support and just check for
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If not, cast it into an array at the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if I'm misunderstanding what you guys are asking. I thought that this And I thought the So I think if I have a PR making this change here against an older version of this branch: Am I misunderstanding something? Is that version basically the change you're asking for? Happy to do it if so!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think supporting both and just using the default is easier. I'll make the change. |
||
|
|
||
| /** | ||
| * Minify the output JSON schema (no whitespace). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export function castArray<T>(input: undefined | T | T[]): undefined | T[] { | ||
| if (input === undefined) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return Array.isArray(input) ? input : [input]; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| type NonExportedType = { | ||
| misc: number; | ||
| }; | ||
|
|
||
| export type ExportedType = { | ||
| val: string; | ||
| val2: NonExportedType; | ||
| }; | ||
|
|
||
| export interface ExportedInterface { | ||
| val: string; | ||
| } | ||
|
|
||
| export type Object1Prop = { | ||
| name: string; | ||
| }; | ||
|
|
||
| export type Object2Prop = { | ||
| description: string; | ||
| }; | ||
|
|
||
| export type MyObject1 = { | ||
| id: number; | ||
| bar: Object1Prop; | ||
| }; | ||
|
|
||
| export type MyObject2 = { | ||
| idStr: string; | ||
| baz: Object2Prop; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| { | ||
| "$schema": "http://json-schema.org/draft-07/schema#", | ||
| "definitions": { | ||
| "MyObject1": { | ||
| "properties": { | ||
| "id": { | ||
| "type": "number" | ||
| }, | ||
| "bar": { | ||
| "$ref": "#/definitions/Object1Prop" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "id", | ||
| "bar" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| }, | ||
| "MyObject2": { | ||
| "properties": { | ||
| "idStr": { | ||
| "type": "string" | ||
| }, | ||
| "baz": { | ||
| "$ref": "#/definitions/Object2Prop" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "idStr", | ||
| "baz" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| }, | ||
| "Object1Prop": { | ||
| "properties": { | ||
| "name": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "name" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| }, | ||
| "Object2Prop": { | ||
| "properties": { | ||
| "description": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "description" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| type NonExportedType = { | ||
| misc: number; | ||
| }; | ||
|
|
||
| export type ExportedType = { | ||
| val: string; | ||
| val2: NonExportedType; | ||
| }; | ||
|
|
||
| export interface ExportedInterface { | ||
| val: string; | ||
| } | ||
|
|
||
| // Exported, so we include it as a root node | ||
| export type Object1Prop = { | ||
| name: string; | ||
| }; | ||
|
|
||
| type Object2Prop = { | ||
| description: string; | ||
| }; | ||
|
|
||
| export type MyObject1 = { | ||
| id: number; | ||
| bar: Object1Prop; | ||
| }; | ||
|
|
||
| export type MyObject2 = { | ||
| idStr: string; | ||
| baz: Object2Prop; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| { | ||
| "$schema": "http://json-schema.org/draft-07/schema#", | ||
| "definitions": { | ||
| "MyObject1": { | ||
| "properties": { | ||
| "id": { | ||
| "type": "number" | ||
| }, | ||
| "bar": { | ||
| "$ref": "#/definitions/Object1Prop" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "id", | ||
| "bar" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| }, | ||
| "MyObject2": { | ||
| "properties": { | ||
| "idStr": { | ||
| "type": "string" | ||
| }, | ||
| "baz": { | ||
| "properties": { | ||
| "description": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "description" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| } | ||
| }, | ||
| "required": [ | ||
| "idStr", | ||
| "baz" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| }, | ||
| "Object1Prop": { | ||
| "properties": { | ||
| "name": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "name" | ||
| ], | ||
| "type": "object", | ||
| "additionalProperties": false | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,20 @@ import type { Config } from "./src/Config.js"; | |
| import { BaseError } from "./src/Error/BaseError.js"; | ||
|
|
||
| import pkg from "./package.json"; | ||
| import { castArray } from "./src/Utils/castArray.js"; | ||
|
|
||
| const args = new Command() | ||
| .option("-p, --path <path>", "Source file path") | ||
| .option("-t, --type <name>", "Type name") | ||
| .option( | ||
| "-t, --type <name>", | ||
| "Type name (can be passed multiple times)", | ||
| (value: string, previous: string[] | undefined) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it will always return an array, then the typings can be a simple
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thinking here was that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we use ... instead as documented in https://github.com/tj/commander.js?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding (and this is just from reading the docs and talking to chatgpt--I don't have any real experience with Although I see now that one of their examples does show both working: They have a section in the docs about ambiguity in parsing variadic arguments that I didn't think applied to the Let me know if you'd prefer I switch to variadic arguments.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like his approach of always returning an array :) |
||
| if (previous) { | ||
| return previous.concat(value); | ||
| } | ||
| return [value]; | ||
| }, | ||
| ) | ||
| .option("-i, --id <name>", "$id for generated schema") | ||
| .option("-f, --tsconfig <path>", "Custom tsconfig.json path") | ||
| .addOption( | ||
|
|
@@ -84,7 +94,7 @@ const config: Config = { | |
| }; | ||
|
|
||
| try { | ||
| const schema = createGenerator(config).createSchema(args.type); | ||
| const schema = createGenerator(config).createSchema(castArray(args.type)); | ||
|
|
||
| const stringify = config.sortProps ? stableStringify : JSON.stringify; | ||
| // need as string since TS can't figure out that the string | undefined case doesn't happen | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.