build: more type information for docs #6947

devversion · 2017-09-08T15:11:56Z

Upgrades dgeni and dgeni-packages to the latest available version (including TypeScript support; and newer TypeScript version for AST)
More warnings if type information is missing on public TypeScript symbol
Adds missing explicit type declarations to all components
Types are now being displayed for accessors and properties (this was broken before somehow)
Rewrote the Dgeni setup in TypeScript for type safety and better integration into the gulp workflow.

Note: Most of the things inside of the existing Dgeni setup is the same as before and I didn't look into other improvements there (e.g logic simplification) because the PR is big enough already.

crisbeto

Code-wise LGTM, but I'm wondering about how are we going to enforce that we add all the necessary typings?

crisbeto · 2017-09-09T07:34:15Z

src/lib/form-field/form-field.ts

@@ -94,7 +94,7 @@ export class MdFormField implements AfterViewInit, AfterContentInit, AfterConten

  /** @deprecated Use `color` instead. */
  @Input()
-  get dividerColor() { return this.color; }
+  get dividerColor(): 'primary' | 'accent' | 'warn' { return this.color; }


We have the ThemePalette type for this already.

Yeah I wanted to use that as well, but it also includes undefined as a possible value, which is not valid here.

crisbeto · 2017-09-09T07:35:05Z

tools/dgeni/common/decorators.ts

+import {PropertyMemberDoc} from 'dgeni-packages/typescript/api-doc-types/PropertyMemberDoc';
+import {MemberDoc} from 'dgeni-packages/typescript/api-doc-types/MemberDoc';
+
+const SELECTOR_BLACKLIST  = new Set([


Some inconsistent spacing here.

crisbeto · 2017-09-09T07:39:12Z

tools/dgeni/processors/categorizer.ts

+import {ClassExportDoc} from 'dgeni-packages/typescript/api-doc-types/ClassExportDoc';
+import {PropertyMemberDoc} from 'dgeni-packages/typescript/api-doc-types/PropertyMemberDoc';
+import {
+  NormalizedMethodMemberDoc, normalizeMethodParameters


The normalizeMethodParameters should be on a separate line.

devversion · 2017-09-09T09:14:11Z

@crisbeto Addressed your feedback and rebased on top of the sort members commit.

Regarding the enforcement: Good point, I have a TSLint rule in mind. We can base those rules on top of the existing rules like typedef (will be something as a follow-up)

devversion · 2017-09-29T12:45:03Z

@jelbourn Rebased this one. Please have another look.

jelbourn · 2017-10-04T00:01:05Z

tools/dgeni/common/decorators.ts

+
+export function isProperty(doc: MemberDoc) {
+  if (doc instanceof PropertyMemberDoc ||
+      // The latest Dgeni version no longer treats getters or setters as properties. In theory


"In theory those" -> "From a user perspective, these"

jelbourn · 2017-10-04T00:04:02Z

tools/dgeni/common/dgeni-accessors-parse.ts

+import {CategorizedPropertyMemberDoc} from '../processors/categorizer';
+
+/**
+ * Accessors are not parsed properly within the latest dgeni-packages package.


Is there an issue to link for this? What about the accessors isn't parsed correctly?

I only reported this on Slack if I recall. Created an issue for it now: angular/dgeni-packages#246

jelbourn · 2017-10-04T00:09:56Z

tools/dgeni/common/dgeni-accessors-parse.ts

+        member.kind === SyntaxKind.SetAccessor)
+      .filter((member: any) => member.name && (member.name as any).text === propertyDoc.name)
+      .forEach((member: GetAccessorDeclaration|SetAccessorDeclaration) => {
+        if (member.type) {


Add comment that explains how you extract the type

jelbourn · 2017-10-04T00:10:41Z

tools/dgeni/common/dgeni-accessors-parse.ts

+    classDeclaration.members
+      .filter((member: any) => member.kind === SyntaxKind.GetAccessor ||
+        member.kind === SyntaxKind.SetAccessor)
+      .filter((member: any) => member.name && (member.name as any).text === propertyDoc.name)


If member is any, why do you need to use as any on member.name?

jelbourn · 2017-10-04T00:12:04Z

tools/dgeni/common/dgeni-accessors-parse.ts

+        // If the current property document doesn't have any description set and the current
+        // TypeScript has a symbol from the TypeChecker, we can try to find a documentation
+        // comment on that symbol.
+        if (!propertyDoc.description && (member as any).symbol) {


If you need the as any here, is GetAccessorDeclaration|SetAccessorDeclaration the right type for member?

Yeah this is the correct type. It just looks like TypeScript isn't exposing the symbol property in their typings. The same is happening on Dgeni as well (see here)

* Upgrades dgeni and dgeni-packages to the latest available version (including TypeScript support; and newer TypeScript version for AST) * More warnings if type information is missing on public TypeScript symbol * Adds missing explicit type declarations to all components * Types are now being displayed for accessors and properties (this was broken before somehow) * Rewrote the Dgeni setup in TypeScript for type safety and better integration into the gulp workflow.

devversion · 2017-10-04T14:15:04Z

@jelbourn Addressed your feedback.

jelbourn

LGTM

angular-automatic-lock-bot · 2019-09-07T08:02:17Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

devversion added the pr: needs review label Sep 8, 2017

devversion assigned jelbourn Sep 8, 2017

devversion requested review from andrewseguin, crisbeto, jelbourn, josephperrott, kara, mmalerba and tinayuangao as code owners September 8, 2017 15:11

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 8, 2017

crisbeto approved these changes Sep 9, 2017

View reviewed changes

devversion force-pushed the build/more-type-info-docs branch from 486bc66 to 2d132f9 Compare September 9, 2017 09:11

willshowell mentioned this pull request Sep 11, 2017

Show types for properties angular/material.angular.io#237

Closed

devversion force-pushed the build/more-type-info-docs branch from 2d132f9 to a97f7f5 Compare September 12, 2017 08:58

devversion added the P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful label Sep 15, 2017

devversion force-pushed the build/more-type-info-docs branch 2 times, most recently from 9b70ae2 to 745357e Compare September 22, 2017 14:42

devversion added P2 The issue is important to a large percentage of users, with a workaround and removed P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful labels Sep 27, 2017

devversion force-pushed the build/more-type-info-docs branch from 745357e to 861c81d Compare September 29, 2017 12:43

devversion force-pushed the build/more-type-info-docs branch 2 times, most recently from a64eb21 to ae76648 Compare October 1, 2017 08:52

willshowell mentioned this pull request Oct 2, 2017

chore(api): overlay class audits #6372 #7454

Closed

jelbourn reviewed Oct 4, 2017

View reviewed changes

devversion force-pushed the build/more-type-info-docs branch from 66114b9 to 52e4712 Compare October 4, 2017 14:14

willshowell mentioned this pull request Oct 4, 2017

Color/disabled mixins removed properties from the api docs #5284

Closed

jelbourn approved these changes Oct 4, 2017

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Oct 4, 2017

kara merged commit 388845c into angular:master Oct 4, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

build: more type information for docs #6947

build: more type information for docs #6947

Uh oh!

Conversation

devversion commented Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion commented Sep 9, 2017

Uh oh!

devversion commented Sep 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion commented Oct 4, 2017

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 7, 2019

Uh oh!

Uh oh!

devversion commented Sep 8, 2017 •

edited

Loading

devversion Oct 4, 2017 •

edited

Loading