-
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
=======================================
Coverage 91.96% 91.97%
=======================================
Files 15 15
Lines 909 910 +1
Branches 265 266 +1
=======================================
+ Hits 836 837 +1
Misses 73 73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| name: string; | ||
| optional: boolean; | ||
| type: string; | ||
| inlineType?: TypeDefinition; |
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
| return { | ||
| type: type, | ||
| inlineType: { | ||
| name: realTypeName.length < 100 ? realTypeName : 'object', |
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.
If a type is anonymous there will be a string like
{ 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?: { ...; } |...
We do not need these.
I checked the longest type name in our system is CollectionPreferencesProps.ContentDisplayPreferenceI18nStrings, 62 chars, still below the limit
| }, | ||
| ]); | ||
| test('should print long inline types', () => { | ||
| expect(button.properties).toMatchSnapshot(); |
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.
This type is too long and updating it manually on every change is tedious, let's do a snapshot test
| isArrayType(realType) || | ||
| realTypeName === 'HTMLElement' | ||
| realTypeName === 'HTMLElement' || | ||
| type === 'React.ReactNode' |
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.
Prevent React types from expanding into boolean | React.ReactChild | React.ReactFragment | React.ReactPortal
| properties: [ | ||
| { | ||
| name: 'content', | ||
| optional: true, |
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.
A keen reviewer might ask why there is optional: true when the content above does not have a ?
This is happening because undefined is assignable to ReactNode, so {media: undefined} is supported, which makes it technically optional
| if ( | ||
| realType.flags & ts.TypeFlags.String || | ||
| realType.flags & ts.TypeFlags.StringLiteral || | ||
| realType.flags & ts.TypeFlags.Literal || |
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.
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"
}
Issue #, if available:
Description of changes:
Follow up for this change #76
Make deeply nested inline types properly printed now
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.