-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Expand long inline types #82
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 all commits
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 |
|---|---|---|
|
|
@@ -22,11 +22,12 @@ export function getObjectDefinition( | |
| const realTypeName = stringifyType(realType, checker); | ||
| if ( | ||
| realType.flags & ts.TypeFlags.String || | ||
| realType.flags & ts.TypeFlags.StringLiteral || | ||
| realType.flags & ts.TypeFlags.Literal || | ||
|
Member
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. Make sure that boolean and number literals do not expand too. Without this line it generates a type definition like this {
"inlineType": {
"name": "() => boolean",
"parameters": [],
"returnType": "boolean",
"type": "function"
},
"name": "valueOf",
"optional": false,
"type": "() => boolean"
}
|
||
| realType.flags & ts.TypeFlags.Boolean || | ||
| realType.flags & ts.TypeFlags.Number || | ||
| isArrayType(realType) || | ||
| realTypeName === 'HTMLElement' | ||
| realTypeName === 'HTMLElement' || | ||
| type === 'React.ReactNode' | ||
|
Member
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. Prevent React types from expanding into |
||
| ) { | ||
| // do not expand built-in Javascript methods or primitive values | ||
| return { type }; | ||
|
|
@@ -38,24 +39,25 @@ export function getObjectDefinition( | |
| const properties = realType | ||
| .getProperties() | ||
| .map(prop => { | ||
| const propType = checker.getTypeAtLocation(extractDeclaration(prop)); | ||
| const propNode = extractDeclaration(prop) as ts.PropertyDeclaration; | ||
| const propType = checker.getTypeAtLocation(propNode); | ||
| const typeString = stringifyType(propType, checker); | ||
|
|
||
| return { | ||
| name: prop.getName(), | ||
| type: stringifyType(propType, checker), | ||
| optional: isOptional(propType), | ||
| ...getObjectDefinition(typeString, propType, propNode.type, checker), | ||
| }; | ||
| }) | ||
| .sort((a, b) => a.name.localeCompare(b.name)); | ||
| if (properties.every(prop => prop.type.length < 200)) { | ||
| return { | ||
| type: type, | ||
| inlineType: { | ||
| name: realTypeName, | ||
| type: 'object', | ||
| properties: properties, | ||
| }, | ||
| }; | ||
| } | ||
| return { | ||
| type: type, | ||
| inlineType: { | ||
| name: realTypeName.length < 100 ? realTypeName : 'object', | ||
|
Member
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. If a type is anonymous there will be a string like We do not need these. I checked the longest type name in our system is |
||
| type: 'object', | ||
| properties: properties, | ||
| }, | ||
| }; | ||
| } | ||
| if (realType.getCallSignatures().length > 0) { | ||
| if (realType.getCallSignatures().length > 1) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html | ||
|
|
||
| exports[`should print long inline types 1`] = ` | ||
| [ | ||
| { | ||
| "analyticsTag": undefined, | ||
| "defaultValue": undefined, | ||
| "deprecatedTag": undefined, | ||
| "description": undefined, | ||
| "i18nTag": undefined, | ||
| "inlineType": { | ||
| "name": "ButtonProps.Style", | ||
| "properties": [ | ||
| { | ||
| "inlineType": { | ||
| "name": "object", | ||
| "properties": [ | ||
| { | ||
| "inlineType": { | ||
| "name": "object", | ||
| "properties": [ | ||
| { | ||
| "name": "active", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "default", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "disabled", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "hover", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| ], | ||
| "type": "object", | ||
| }, | ||
| "name": "background", | ||
| "optional": true, | ||
| "type": "{ active?: string | undefined; default?: string | undefined; disabled?: string | undefined; hover?: string | undefined; }", | ||
| }, | ||
| { | ||
| "inlineType": { | ||
| "name": "object", | ||
| "properties": [ | ||
| { | ||
| "name": "active", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "default", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "disabled", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "hover", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| ], | ||
| "type": "object", | ||
| }, | ||
| "name": "borderColor", | ||
| "optional": true, | ||
| "type": "{ active?: string | undefined; default?: string | undefined; disabled?: string | undefined; hover?: string | undefined; }", | ||
| }, | ||
| { | ||
| "inlineType": { | ||
| "name": "object", | ||
| "properties": [ | ||
| { | ||
| "name": "active", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "default", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "disabled", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| { | ||
| "name": "hover", | ||
| "optional": true, | ||
| "type": "string", | ||
| }, | ||
| ], | ||
| "type": "object", | ||
| }, | ||
| "name": "color", | ||
| "optional": true, | ||
| "type": "{ active?: string | undefined; default?: string | undefined; disabled?: string | undefined; hover?: string | undefined; }", | ||
| }, | ||
| ], | ||
| "type": "object", | ||
| }, | ||
| "name": "root", | ||
| "optional": true, | ||
| "type": "{ background?: { active?: string | undefined; default?: string | undefined; disabled?: string | undefined; hover?: string | undefined; } | undefined; color?: { active?: string | undefined; default?: string | undefined; disabled?: string | undefined; hover?: string | undefined; } | undefined; borderColor?: { ...; } |...", | ||
| }, | ||
| ], | ||
| "type": "object", | ||
| }, | ||
| "name": "style", | ||
| "optional": false, | ||
| "systemTags": undefined, | ||
| "type": "ButtonProps.Style", | ||
| "visualRefreshTag": undefined, | ||
| }, | ||
| ] | ||
| `; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,17 @@ test('should have correct property types', () => { | |
| name: 'allItemsSelectionLabel', | ||
| optional: true, | ||
| type: '((data: TableProps.SelectionState<T>) => string)', | ||
| inlineType: { | ||
| name: '(data: TableProps.SelectionState<T>) => string', | ||
| parameters: [ | ||
| { | ||
| name: 'data', | ||
| type: 'TableProps.SelectionState<T>', | ||
| }, | ||
| ], | ||
| returnType: 'string', | ||
| type: 'function', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
|
|
@@ -149,7 +160,13 @@ test('should properly display string union types', () => { | |
| { | ||
| name: 'type', | ||
| optional: true, | ||
| type: '"link" | "link-group" | "expandable-link-group"', | ||
| type: 'string', | ||
| inlineType: { | ||
| name: '"link" | "link-group" | "expandable-link-group"', | ||
| type: 'union', | ||
| valueDescriptions: undefined, | ||
| values: ['link', 'link-group', 'expandable-link-group'], | ||
| }, | ||
| }, | ||
| ], | ||
| }); | ||
|
|
@@ -186,6 +203,33 @@ test('should parse string literal type as single-value union', () => { | |
| type: 'ReadonlyArray<ButtonGroupProps.Item>', | ||
| optional: false, | ||
| }, | ||
| { | ||
| name: 'mainAction', | ||
| description: 'Main action for the group', | ||
| type: '{ alwaysFalse: false; alwaysOne: 1; alwaysSomething: "something"; }', | ||
| optional: true, | ||
| inlineType: { | ||
| name: '{ alwaysFalse: false; alwaysOne: 1; alwaysSomething: "something"; }', | ||
| type: 'object', | ||
| properties: [ | ||
| { | ||
| name: 'alwaysFalse', | ||
| optional: false, | ||
| type: 'false', | ||
| }, | ||
| { | ||
| name: 'alwaysOne', | ||
| optional: false, | ||
| type: '1', | ||
| }, | ||
| { | ||
| name: 'alwaysSomething', | ||
| optional: false, | ||
| type: '"something"', | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| { | ||
| name: 'variant', | ||
| description: 'This is variant', | ||
|
|
@@ -195,12 +239,6 @@ test('should parse string literal type as single-value union', () => { | |
| ]); | ||
| }); | ||
|
|
||
| test('should trim long inline types', () => { | ||
| expect(button.properties).toEqual([ | ||
| { | ||
| name: 'style', | ||
| type: 'ButtonProps.Style', | ||
| optional: false, | ||
| }, | ||
| ]); | ||
| test('should print long inline types', () => { | ||
| expect(button.properties).toMatchSnapshot(); | ||
|
Member
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. This type is too long and updating it manually on every change is tedious, let's do a snapshot test |
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,25 @@ beforeAll(() => { | |
| }); | ||
|
|
||
| test('should have correct region definitions', () => { | ||
| expect(component.properties).toEqual([]); | ||
| expect(component.properties).toEqual([ | ||
| { | ||
| name: 'media', | ||
| description: 'Media content', | ||
| optional: true, | ||
| type: '{ content: React.ReactNode; }', | ||
| inlineType: { | ||
| name: '{ content: React.ReactNode; }', | ||
| properties: [ | ||
| { | ||
| name: 'content', | ||
| optional: true, | ||
|
Member
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. A keen reviewer might ask why there is This is happening because |
||
| type: 'React.ReactNode', | ||
| }, | ||
| ], | ||
| type: 'object', | ||
| }, | ||
| }, | ||
| ]); | ||
| expect(component.regions).toEqual([ | ||
| { | ||
| name: 'children', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is recursive now