-
Notifications
You must be signed in to change notification settings - Fork 818
feat(typescript): update to typescript 4.3.5 #3103
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 6 commits
02a06ee
ad2c239
308d16b
e17ec84
f71caa2
a8d30d6
aa39cb1
8404abd
98bfd96
d0ab630
aedcc28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||
| import { mapJSDocTagInfo } from '../transform-utils'; | ||||||||
|
|
||||||||
| describe('transform utils', () => { | ||||||||
| it('flattens TypeScript JSDocTagInfo to Stencil JSDocTagInfo', () => { | ||||||||
| const tags = [ | ||||||||
| { | ||||||||
| name: 'param', | ||||||||
| text: [ | ||||||||
| { kind: 'text', text: 'foo' }, | ||||||||
| { kind: 'space', text: ' ' }, | ||||||||
| { kind: 'text', text: 'the first parameter' }, | ||||||||
| ], | ||||||||
| }, | ||||||||
| { | ||||||||
| name: 'param', | ||||||||
| text: [ | ||||||||
| { kind: 'text', text: 'bar' }, | ||||||||
| { kind: 'space', text: ' ' }, | ||||||||
| { kind: 'text', text: 'the second parameter' }, | ||||||||
| ], | ||||||||
| }, | ||||||||
| ]; | ||||||||
|
|
||||||||
| expect(mapJSDocTagInfo(tags)).toEqual([ | ||||||||
| { name: 'param', text: 'foo the first parameter' }, | ||||||||
|
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. To coincide with my suggestion above:
Suggested change
|
||||||||
| { name: 'param', text: 'bar the second parameter' }, | ||||||||
| ]); | ||||||||
| }); | ||||||||
| }); | ||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -515,11 +515,20 @@ export const serializeSymbol = (checker: ts.TypeChecker, symbol: ts.Symbol): d.C | |||||
| }; | ||||||
| } | ||||||
| return { | ||||||
| tags: symbol.getJsDocTags().map((tag) => ({ text: tag.text, name: tag.name })), | ||||||
| tags: mapJSDocTagInfo(symbol.getJsDocTags()), | ||||||
| text: ts.displayPartsToString(symbol.getDocumentationComment(checker)), | ||||||
| }; | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * Maps a TypeScript 4.3+ JSDocTagInfo to a flattened Stencil CompilerJsDocTagInfo. | ||||||
| * @param tags An array of JSDocTagInfo objects. | ||||||
| * @return An array of CompilerJsDocTagInfo objects. | ||||||
| */ | ||||||
| export const mapJSDocTagInfo = (tags: ts.JSDocTagInfo[]): d.CompilerJsDocTagInfo[] => { | ||||||
|
||||||
| export const mapJSDocTagInfo = (tags: ts.JSDocTagInfo[]): d.CompilerJsDocTagInfo[] => { | |
| export const mapJSDocTagInfo = (tags: ReadonlyArray<ts.JSDocTagInfo>): d.CompilerJsDocTagInfo[] => { |
(I don't think we can do the same with the return type, because that would lead to changes in other functions, like transform-utils.ts#mapJSDocTagInfo)
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.
TIL - opted for the equivalent readonly ts.JSDocTagInfo[].
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.
When we have >1 SymbolDisplayPart in a JSDocTagInfo, we're joining them in this change with .join('') - can we add a comment as to why we need to join them? From my own playing around it seems to be related to how the pieces of the JSDoc are structured internally. For example I add some 'doc' to a boilerplate Stencil component library:
/**
* The middle name
* @argument i love to argue
* @async nsync backstreet boys
*/
@Prop() middle: string;Which internally becomes:
{ text: 'i', kind: 'parameterName' }
{ text: ' ', kind: 'space' }
{ text: 'love to argue', kind: 'text' }
{ text: 'nsync backstreet boys', kind: 'text' }
When we create a JSON readme, this all looks right to me:
{
"name": "middle",
"type": "string",
"mutable": false,
"attr": "middle",
"reflectToAttr": false,
"docs": "The middle name",
"docsTags": [
{
"name": "argument",
"text": "i love to argue"
},
{
"name": "async",
"text": "nsync backstreet boys"
}
],
"values": [
{
"type": "string"
}
],
"optional": false,
"required": false
}
],Is my understanding right there?
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.
Exactly. I've added a comment describing this.
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.
Nit: Can we add a test for no/empty text?