Skip to content

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

Merged
merged 11 commits into from
Oct 19, 2021
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"sizzle": "^2.3.6",
"terser": "5.6.1",
"tslib": "^2.1.0",
"typescript": "4.2.3",
"typescript": "4.3.5",
"webpack": "^4.46.0",
"ws": "7.4.6"
},
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/sys/dependencies.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@
"compiler/lib.es2020.sharedmemory.d.ts",
"compiler/lib.es2020.string.d.ts",
"compiler/lib.es2020.symbol.wellknown.d.ts",
"compiler/lib.es2021.d.ts",
"compiler/lib.es2021.full.d.ts",
"compiler/lib.es2021.promise.d.ts",
"compiler/lib.es2021.string.d.ts",
"compiler/lib.es2021.weakref.d.ts",
"compiler/lib.es5.d.ts",
"compiler/lib.es6.d.ts",
"compiler/lib.esnext.d.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
createStaticGetter,
getAttributeTypeInfo,
isMemberPrivate,
mapJSDocTagInfo,
serializeSymbol,
typeToString,
validateReferences,
Expand Down Expand Up @@ -95,7 +96,7 @@ const parseMethodDecorator = (
},
docs: {
text: ts.displayPartsToString(signature.getDocumentationComment(typeChecker)),
tags: signature.getJsDocTags(),
tags: mapJSDocTagInfo(signature.getJsDocTags()),
},
};
validateReferences(diagnostics, methodMeta.complexType.references, method.type || method.name);
Expand Down
41 changes: 41 additions & 0 deletions src/compiler/transformers/test/transform-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { mapJSDocTagInfo } from '../transform-utils';

describe('transform utils', () => {
it('flattens TypeScript JSDocTagInfo to Stencil JSDocTagInfo', () => {
// tags corresponds to the following JSDoc
/*
* @param foo the first parameter
* @param bar
* @returns
* @see {@link https://example.com}
*/
const tags = [
{
Copy link
Contributor

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?

Suggested change
{
{
name: 'param',
text: [],
},
{

name: 'param',
text: [
{ text: 'foo', kind: 'parameterName' },
{ text: ' ', kind: 'space' },
{ text: 'the first parameter', kind: 'text' },
],
},
{ name: 'param', text: [{ text: 'bar', kind: 'text' }] },
{ name: 'returns', text: undefined },
{
name: 'see',
text: [
{ text: '', kind: 'text' },
{ text: '{@link ', kind: 'link' },
{ text: 'https://example.com', kind: 'linkText' },
{ text: '}', kind: 'link' },
],
},
];

expect(mapJSDocTagInfo(tags)).toEqual([
{ name: 'param', text: 'foo the first parameter' },
Copy link
Contributor

Choose a reason for hiding this comment

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

To coincide with my suggestion above:

Suggested change
{ name: 'param', text: 'foo the first parameter' },
{ name: 'param', text: '' },
{ name: 'param', text: 'foo the first parameter' },

{ name: 'param', text: 'bar' },
{ name: 'returns', text: undefined },
{ name: 'see', text: '{@link https://example.com}' },
]);
});
});
14 changes: 13 additions & 1 deletion src/compiler/transformers/transform-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,23 @@ 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 A readonly array of JSDocTagInfo objects.
* @returns An array of CompilerJsDocTagInfo objects.
*/
export const mapJSDocTagInfo = (tags: readonly ts.JSDocTagInfo[]): d.CompilerJsDocTagInfo[] => {
// The text following a tag is split semantically by TS 4.3+, e.g. '@param foo the first parameter' ->
// [{text: 'foo', kind: 'parameterName'}, {text: ' ', kind: 'space'}, {text: 'the first parameter', kind: 'text'}], so
// we join the elements to reconstruct the text.
return tags.map((tag) => ({ ...tag, text: tag.text?.map((part) => part.text).join('') }));
Copy link
Contributor

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?

Copy link
Contributor Author

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.

};

export const serializeDocsSymbol = (checker: ts.TypeChecker, symbol: ts.Symbol) => {
const type = checker.getTypeOfSymbolAtLocation(symbol, symbol.valueDeclaration);
const set = new Set<string>();
Expand Down
6 changes: 3 additions & 3 deletions src/mock-doc/comment-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ export class MockComment extends MockNode {
super(ownerDocument, NODE_TYPES.COMMENT_NODE, NODE_NAMES.COMMENT_NODE, data);
}

cloneNode(_deep?: boolean) {
override cloneNode(_deep?: boolean) {
return new MockComment(null, this.nodeValue);
}

get textContent() {
override get textContent() {
return this.nodeValue;
}

set textContent(text) {
override set textContent(text) {
this.nodeValue = text;
}
}
2 changes: 1 addition & 1 deletion src/mock-doc/document-fragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class MockDocumentFragment extends MockHTMLElement {
return getElementById(this, id);
}

cloneNode(deep?: boolean) {
override cloneNode(deep?: boolean) {
const cloned = new MockDocumentFragment(null);

if (deep) {
Expand Down
10 changes: 5 additions & 5 deletions src/mock-doc/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ export class MockDocument extends MockHTMLElement {
}
}

get dir() {
override get dir() {
return this.documentElement.dir;
}
set dir(value: string) {
override set dir(value: string) {
this.documentElement.dir = value;
}

Expand Down Expand Up @@ -166,7 +166,7 @@ export class MockDocument extends MockHTMLElement {
}
}

appendChild(newNode: MockElement) {
override appendChild(newNode: MockElement) {
newNode.remove();
newNode.parentNode = this;
this.childNodes.push(newNode);
Expand Down Expand Up @@ -222,14 +222,14 @@ export class MockDocument extends MockHTMLElement {
return getElementsByName(this, elmName.toLowerCase());
}

get title() {
override get title() {
const title = this.head.childNodes.find((elm) => elm.nodeName === 'TITLE') as MockElement;
if (title != null && typeof title.textContent === 'string') {
return title.textContent.trim();
}
return '';
}
set title(value: string) {
override set title(value: string) {
const head = this.head;
let title = head.childNodes.find((elm) => elm.nodeName === 'TITLE') as MockElement;
if (title == null) {
Expand Down
22 changes: 11 additions & 11 deletions src/mock-doc/element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ export class MockImageElement extends MockHTMLElement {
super(ownerDocument, 'img');
}

get draggable() {
override get draggable() {
return this.getAttributeNS(null, 'draggable') !== 'false';
}
set draggable(value: boolean) {
override set draggable(value: boolean) {
this.setAttributeNS(null, 'draggable', value);
}

Expand Down Expand Up @@ -243,24 +243,24 @@ export class MockStyleElement extends MockHTMLElement {
this.sheet = new MockCSSStyleSheet(this);
}

get innerHTML() {
override get innerHTML() {
return getStyleElementText(this);
}
set innerHTML(value: string) {
override set innerHTML(value: string) {
setStyleElementText(this, value);
}

get innerText() {
override get innerText() {
return getStyleElementText(this);
}
set innerText(value: string) {
override set innerText(value: string) {
setStyleElementText(this, value);
}

get textContent() {
override get textContent() {
return getStyleElementText(this);
}
set textContent(value: string) {
override set textContent(value: string) {
setStyleElementText(this, value);
}
}
Expand Down Expand Up @@ -318,14 +318,14 @@ export class MockTemplateElement extends MockHTMLElement {
this.content = new MockDocumentFragment(ownerDocument);
}

get innerHTML() {
override get innerHTML() {
return this.content.innerHTML;
}
set innerHTML(html: string) {
override set innerHTML(html: string) {
this.content.innerHTML = html;
}

cloneNode(deep?: boolean) {
override cloneNode(deep?: boolean) {
const cloned = new MockTemplateElement(null);
cloned.attributes = cloneAttributes(this.attributes);

Expand Down
Loading