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:/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:/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:/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:/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:/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:/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:/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