Review feedback updates 2

packages/kbn-docs-utils/src/api_docs/build_api_declarations/build_api_declaration.ts

@@ -31,9 +31,11 @@ import { getTypeKind } from './get_type_kind';
  * @param plugins The list of plugins registered is used for building cross plugin links by looking up
  * the plugin by import path. We could accomplish the same thing via a regex on the import path, but this lets us
  * decouple plugin path from plugin id.
- * @param parentAnchorLink Used to build a nested id for the API. If this is a top level plugin API, parentAnchorLink.apiId
- * will be undefined.
  * @param log Logs messages to console.
+ * @param pluginName The name of the plugin this declaration belongs to.
+ * @param scope The scope this declaration belongs to (server, public, or common).
+ * @param parentApiId If this declaration is nested inside another declaration, it should have a parent id. This
+ * is used to create the anchor link to this API item.
  * @param name An optional name to pass through which will be used instead of node.getName, if it
  * exists. For some types, like Parameters, the name comes on the parent node, but we want the doc def
  * to be built from the TypedNode

packages/kbn-docs-utils/src/api_docs/build_api_declarations/extract_import_refs.test.ts

@@ -61,7 +61,7 @@ it('test extractImportReference with public folder nested under server folder',
 
 it('test extractImportReference with two imports', () => {
   const results = extractImportReferences(
-    `<I extends import("${plugin.directory}/public/foo").FooFoo, O extends import("${plugin.directory}/public/bar").Bar>`,
+    `<I extends import("${plugin.directory}/public/foo/index").FooFoo, O extends import("${plugin.directory}/public/bar").Bar>`,
     plugins,
     log
   );
@@ -75,7 +75,7 @@ it('test extractImportReference with two imports', () => {
 
 it('test extractImportReference with unknown imports', () => {
   const results = extractImportReferences(
-    `<I extends import("/plugin_c/public/foo").FooFoo>`,
+    `<I extends import("/plugin_c/public/foo/index").FooFoo>`,
     plugins,
     log
   );
@@ -87,7 +87,7 @@ it('test extractImportReference with unknown imports', () => {
 
 it('test single link', () => {
   const results = extractImportReferences(
-    `import("${plugin.directory}/public/foo").FooFoo`,
+    `import("${plugin.directory}/public/foo/index").FooFoo`,
     plugins,
     log
   );

packages/kbn-docs-utils/src/api_docs/build_api_declarations/get_signature.ts

@@ -7,7 +7,7 @@
  */
 
 import { KibanaPlatformPlugin, ToolingLog } from '@kbn/dev-utils';
-import { Node } from 'ts-morph';
+import { Node, Type } from 'ts-morph';
 import { isNamedNode } from '../tsmorph_utils';
 import { Reference } from '../types';
 import { extractImportReferences } from './extract_import_refs';
@@ -35,18 +35,7 @@ export function getSignature(
   // `export type Foo = string | number;` would show up with a signagure of `Foo` that is a link to itself, instead of
   //  `string | number`.
   if (Node.isTypeAliasDeclaration(node)) {
-    const symbol = node.getSymbol();
-    if (symbol) {
-      const declarations = symbol.getDeclarations();
-      if (declarations.length === 1) {
-        // Unfortunately we are losing some reference links here.
-        signature = declarations[0].getType().getText(node);
-      } else {
-        log.error(
-          `Node is type alias declaration with more than one declaration. This is not handled!`
-        );
-      }
-    }
+    signature = getSignatureForTypeAlias(node.getType(), log, node);
   } else if (Node.isFunctionDeclaration(node)) {
     // See https://github.com/dsherret/ts-morph/issues/907#issue-770284331.
     // Unfortunately this has to be manually pieced together, or it comes up as "typeof TheFunction"
@@ -74,6 +63,7 @@ export function getSignature(
   if (getTypeKind(node).toString() === signature) return undefined;
 
   const referenceLinks = extractImportReferences(signature, plugins, log);
+
   // Don't return the signature if it's a single self referential link.
   if (
     isNamedNode(node) &&
@@ -86,3 +76,42 @@ export function getSignature(
 
   return referenceLinks;
 }
+
+/**
+ * Not all types are handled here, but does return links for the more common ones.
+ */
+function getSignatureForTypeAlias(type: Type, log: ToolingLog, node?: Node): string {
+  if (type.isUnion()) {
+    return type
+      .getUnionTypes()
+      .map((nestedType) => getSignatureForTypeAlias(nestedType, log))
+      .join(' | ');
+  } else if (node && type.getCallSignatures().length >= 1) {
+    return type
+      .getCallSignatures()
+      .map((sig) => {
+        const params = sig
+          .getParameters()
+          .map((p) => `${p.getName()}: ${p.getTypeAtLocation(node).getText()}`)
+          .join(', ');
+        const returnType = sig.getReturnType().getText();
+        return `(${params}) => ${returnType}`;
+      })
+      .join(' ');
+  } else if (node) {
+    const symbol = node.getSymbol();
+    if (symbol) {
+      const declarations = symbol
+        .getDeclarations()
+        .map((d) => d.getType().getText(node))
+        .join(' ');
+      if (symbol.getDeclarations().length !== 1) {
+        log.error(
+          `Node is type alias declaration with more than one declaration. This is not handled! ${declarations} and node is ${node.getText()}`
+        );
+      }
+      return declarations;
+    }
+  }
+  return type.getText();
+}

packages/kbn-docs-utils/src/api_docs/build_api_declarations/js_doc_utils.ts

@@ -21,7 +21,7 @@ export function getCommentsFromNode(node: Node): TextWithLinks | undefined {
     comments = getTextWithLinks(
       node
         .getLeadingCommentRanges()
-        .map((c) => c.getText() + '\n')
+        .map((c) => c.getText())
         .join('\n')
     );
   }

packages/kbn-docs-utils/src/api_docs/get_plugin_api.ts

@@ -7,7 +7,7 @@
  */
 
 import Path from 'path';
-import { Node, Project } from 'ts-morph';
+import { Node, Project, Type } from 'ts-morph';
 import { ToolingLog, KibanaPlatformPlugin } from '@kbn/dev-utils';
 import { ApiScope, Lifecycle } from './types';
 import { ApiDeclaration, PluginApi } from './types';
@@ -50,7 +50,7 @@ function getDeclarations(
 ): ApiDeclaration[] {
   const nodes = getDeclarationNodesForPluginScope(project, plugin, scope, log);
 
-  const contractTypes = getContractTypeNames(project, plugin, scope);
+  const contractTypes = getContractTypes(project, plugin, scope);
 
   const declarations = nodes.reduce<ApiDeclaration[]>((acc, node) => {
     const apiDec = buildApiDeclaration(node, plugins, log, plugin.manifest.id, scope);
@@ -81,15 +81,15 @@ function getDeclarations(
  */
 function getLifecycle(
   node: Node,
-  contractTypeNames: { start?: string; setup?: string }
+  contractTypeNames: { start?: Type; setup?: Type }
 ): Lifecycle | undefined {
   // Note this logic is not tested if a plugin uses "as",
   // like export { Setup as MyPluginSetup } from ..."
-  if (contractTypeNames.start && node.getType().getText() === contractTypeNames.start) {
+  if (contractTypeNames.start && node.getType() === contractTypeNames.start) {
     return Lifecycle.START;
   }
 
-  if (contractTypeNames.setup && node.getType().getText() === contractTypeNames.setup) {
+  if (contractTypeNames.setup && node.getType() === contractTypeNames.setup) {
     return Lifecycle.SETUP;
   }
 }
@@ -103,12 +103,12 @@ function getLifecycle(
  * @returns the name of the two types used for Start and Setup contracts, if they
  * exist and were exported from the plugin class.
  */
-function getContractTypeNames(
+function getContractTypes(
   project: Project,
   plugin: KibanaPlatformPlugin,
   scope: ApiScope
-): { setup?: string; start?: string } {
-  const contractTypes: { setup?: string; start?: string } = {};
+): { setup?: Type; start?: Type } {
+  const contractTypes: { setup?: Type; start?: Type } = {};
   const file = getSourceFileMatching(
     project,
     Path.join(`${plugin.directory}`, scope.toString(), 'plugin.ts')
@@ -122,9 +122,9 @@ function getContractTypeNames(
           .forEach((arg) => {
             // Setup type comes first
             if (index === 0) {
-              contractTypes.setup = arg.getText();
+              contractTypes.setup = arg;
             } else if (index === 1) {
-              contractTypes.start = arg.getText();
+              contractTypes.start = arg;
             }
             index++;
           });

packages/kbn-docs-utils/src/api_docs/mdx/split_apis_by_folder.test.ts

@@ -38,14 +38,14 @@ beforeAll(() => {
 });
 
 test('foo service has all exports', () => {
-  expect(doc?.client.length).toBe(28);
+  expect(doc?.client.length).toBe(32);
   const split = splitApisByFolder(doc);
   expect(split.length).toBe(2);
 
   const fooDoc = split.find((d) => d.id === 'pluginA.foo');
   const mainDoc = split.find((d) => d.id === 'pluginA');
 
   expect(fooDoc?.common.length).toBe(1);
-  expect(fooDoc?.client.length).toBe(1);
-  expect(mainDoc?.client.length).toBe(27);
+  expect(fooDoc?.client.length).toBe(2);
+  expect(mainDoc?.client.length).toBe(30);
 });

packages/kbn-docs-utils/src/api_docs/mdx/write_plugin_split_by_folder.test.ts

@@ -55,7 +55,6 @@ export interface Zed = { zed: string }`
   ];
 
   const doc = getPluginApi(project, plugins[0], plugins, log);
-
   const docs = splitApisByFolder(doc);
 
   // The api at the main level, and one on a service level.

packages/kbn-docs-utils/src/api_docs/mdx/write_plugin_split_by_folder.ts

@@ -45,6 +45,7 @@ function addSection(
   serviceFolders: readonly string[]
 ) {
   const serviceFolderName = getServiceForPath(dec.source.path);
+
   if (serviceFolderName && serviceFolders.find((f) => f === serviceFolderName)) {
     const service = snakeToCamel(serviceFolderName);
     if (!pluginServices[service]) {

packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/common/index.ts

@@ -7,3 +7,7 @@
  */
 
 export { commonFoo } from './foo';
+
+export interface ImACommonType {
+  goo: number;
+}

packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/fns.ts

@@ -74,3 +74,5 @@ interface ImNotExported {
 }
 
 export const fnWithNonExportedRef = (a: ImNotExported) => 'shi';
+
+export type NotAnArrowFnType = typeof notAnArrowFn;

packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts

@@ -7,3 +7,7 @@
  */
 
 export const doTheFooFnThing = () => {};
+
+export type FooType = () => 'foo';
+
+export type ImNotExportedFromIndex = () => { bar: string };

packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/index.ts

@@ -9,7 +9,7 @@
 import { PluginA, Setup, Start, SearchSpec } from './plugin';
 export { Setup, Start, SearchSpec };
 
-export { doTheFooFnThing } from './foo';
+export { doTheFooFnThing, FooType } from './foo';
 
 export * from './fns';
 export * from './classes';

packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/types.ts

@@ -6,14 +6,17 @@
  * Side Public License, v 1.
  */
 
+import { ImACommonType } from '../common';
+import { FooType, ImNotExportedFromIndex } from './foo';
+
 /**
  * How should a potentially undefined type show up.
  */
 export type StringOrUndefinedType = string | undefined;
 
 export type TypeWithGeneric<T> = T[];
 
-export type ImAType = string | number | TypeWithGeneric<string>;
+export type ImAType = string | number | TypeWithGeneric<string> | FooType | ImACommonType;
 
 /**
  * This is a type that defines a function.
@@ -30,3 +33,10 @@ export enum DayOfWeek {
   FRIDAY, // How about this comment, hmmm?
   SATURDAY,
 }
+
+/**
+ * Calling node.getSymbol().getDeclarations() will return > 1 declaration.
+ */
+export type MultipleDeclarationsType = TypeWithGeneric<typeof DayOfWeek>;
+
+export type IRefANotExportedType = ImNotExportedFromIndex | { zed: 'hi' };

packages/kbn-docs-utils/src/api_docs/tests/api_doc_suite.test.ts

@@ -13,7 +13,7 @@ import { Project } from 'ts-morph';
 import { ToolingLog, KibanaPlatformPlugin } from '@kbn/dev-utils';
 
 import { writePluginDocs } from '../mdx/write_plugin_mdx_docs';
-import { ApiDeclaration, PluginApi, TextWithLinks, TypeKind } from '../types';
+import { ApiDeclaration, PluginApi, Reference, TextWithLinks, TypeKind } from '../types';
 import { getKibanaPlatformPlugin } from './kibana_platform_plugin_mock';
 import { getPluginApi } from '../get_plugin_api';
 import { groupPluginApi } from '../utils';
@@ -88,6 +88,7 @@ beforeAll(() => {
   const plugins: KibanaPlatformPlugin[] = [pluginA];
 
   doc = getPluginApi(project, plugins[0], plugins, log);
+
   mdxOutputFolder = Path.resolve(__dirname, 'snapshots');
   writePluginDocs(mdxOutputFolder, doc, log);
 });
@@ -185,8 +186,7 @@ describe('objects', () => {
       Array [
         "/**
          * The docs should show this inline comment.
-         */
-      ",
+         */",
       ]
     `);
 
@@ -246,13 +246,56 @@ describe('Misc types', () => {
     expect(fnType?.type).toBe(TypeKind.TypeKind);
     expect(fnType?.signature!).toMatchInlineSnapshot(`
       Array [
-        "<T>(t: T) => TypeWithGeneric<T>",
+        "(t: T) => ",
+        Object {
+          "docId": "kibPluginAPluginApi",
+          "pluginId": "pluginA",
+          "scope": "public",
+          "section": "def-public.TypeWithGeneric",
+          "text": "TypeWithGeneric",
+        },
+        "<T>",
+      ]
+    `);
+    expect(linkCount(fnType?.signature!)).toBe(1);
+  });
+
+  it('Union type is exported correctly', () => {
+    const type = doc.client.find((c) => c.label === 'ImAType');
+    expect(type).toBeDefined();
+    expect(type?.type).toBe(TypeKind.TypeKind);
+    expect(type?.signature).toBeDefined();
+    expect(type?.signature!).toMatchInlineSnapshot(`
+      Array [
+        "string | number | ",
+        Object {
+          "docId": "kibPluginAFooPluginApi",
+          "pluginId": "pluginA",
+          "scope": "public",
+          "section": "def-public.FooType",
+          "text": "FooType",
+        },
+        " | ",
+        Object {
+          "docId": "kibPluginAPluginApi",
+          "pluginId": "pluginA",
+          "scope": "public",
+          "section": "def-public.TypeWithGeneric",
+          "text": "TypeWithGeneric",
+        },
+        "<string> | ",
+        Object {
+          "docId": "kibPluginAPluginApi",
+          "pluginId": "pluginA",
+          "scope": "common",
+          "section": "def-common.ImACommonType",
+          "text": "ImACommonType",
+        },
       ]
     `);
 
-    // This is a known bug, links are not captured. https://github.com/dsherret/ts-morph/issues/923
-    // TODO: if we can fix this bug, uncomment this line.
-    // expect(linkCount(fnType?.signature!)).toBe(1);
+    expect(linkCount(type?.signature!)).toBe(3);
+    expect((type!.signature![1] as Reference).docId).toBe('kibPluginAFooPluginApi');
   });
 });
 

packages/kbn-docs-utils/src/api_docs/tests/snapshots/plugin_a.mdx

@@ -28,3 +28,7 @@ import pluginAObj from './plugin_a.json';
 <DocDefinitionList data={pluginAObj.client.enums}/>
 ### Consts, variables and types
 <DocDefinitionList data={pluginAObj.client.misc}/>
+## Common
+
+### Interfaces
+<DocDefinitionList data={pluginAObj.common.interfaces}/>

packages/kbn-docs-utils/src/api_docs/tests/snapshots/plugin_a_foo.json

@@ -1 +1 @@
-{"id":"pluginA.foo","client":{"classes":[],"functions":[{"id":"def-public.doTheFooFnThing","type":"Function","children":[],"signature":["() => void"],"description":[],"label":"doTheFooFnThing","source":{"path":"/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts","lineNumber":9,"link":"https://github.com/elastic/kibana/tree/master/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts#L9"},"returnComment":[],"initialIsOpen":false}],"interfaces":[],"enums":[],"misc":[],"objects":[]},"server":{"classes":[],"functions":[],"interfaces":[],"enums":[],"misc":[],"objects":[]},"common":{"classes":[],"functions":[],"interfaces":[],"enums":[],"misc":[{"id":"def-common.commonFoo","type":"string","label":"commonFoo","description":[],"source":{"path":"/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/common/foo/index.ts","lineNumber":9,"link":"https://github.com/elastic/kibana/tree/master/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/common/foo/index.ts#L9"},"signature":["\"COMMON VAR!\""],"initialIsOpen":false}],"objects":[]}}
+{"id":"pluginA.foo","client":{"classes":[],"functions":[{"id":"def-public.doTheFooFnThing","type":"Function","children":[],"signature":["() => void"],"description":[],"label":"doTheFooFnThing","source":{"path":"/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts","lineNumber":9,"link":"https://github.com/elastic/kibana/tree/master/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts#L9"},"returnComment":[],"initialIsOpen":false}],"interfaces":[],"enums":[],"misc":[{"id":"def-public.FooType","type":"Type","label":"FooType","description":[],"source":{"path":"/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts","lineNumber":11,"link":"https://github.com/elastic/kibana/tree/master/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/public/foo/index.ts#L11"},"signature":["() => \"foo\""],"initialIsOpen":false}],"objects":[]},"server":{"classes":[],"functions":[],"interfaces":[],"enums":[],"misc":[],"objects":[]},"common":{"classes":[],"functions":[],"interfaces":[],"enums":[],"misc":[{"id":"def-common.commonFoo","type":"string","label":"commonFoo","description":[],"source":{"path":"/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/common/foo/index.ts","lineNumber":9,"link":"https://github.com/elastic/kibana/tree/master/packages/kbn-docs-utils/src/api_docs/tests/__fixtures__/src/plugin_a/common/foo/index.ts#L9"},"signature":["\"COMMON VAR!\""],"initialIsOpen":false}],"objects":[]}}

packages/kbn-docs-utils/src/api_docs/tests/snapshots/plugin_a_foo.mdx

@@ -14,6 +14,8 @@ import pluginAFooObj from './plugin_a_foo.json';
 
 ### Functions
 <DocDefinitionList data={pluginAFooObj.client.functions}/>
+### Consts, variables and types
+<DocDefinitionList data={pluginAFooObj.client.misc}/>
 ## Common
 
 ### Consts, variables and types