Skip to content

Commit

Permalink
Do not erase jsdoc comments from nodes that were renamed because of n…
Browse files Browse the repository at this point in the history
…ame collisions

Fixes #298
  • Loading branch information
timocov committed Jan 29, 2024
1 parent ff28f20 commit 2b1b281
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 11 deletions.
19 changes: 12 additions & 7 deletions src/bundle-generator.ts
Expand Up @@ -36,7 +36,7 @@ import {
ModuleType,
} from './module-info';

import { generateOutput, ModuleImportsSet, OutputInputData } from './generate-output';
import { generateOutput, ModuleImportsSet, OutputInputData, StatementSettings } from './generate-output';

import {
normalLog,
Expand Down Expand Up @@ -1141,9 +1141,9 @@ export function generateDtsBundle(entries: readonly EntryPointConfig[], options:
}
},
// eslint-disable-next-line complexity
shouldStatementHasExportKeyword: (statement: ts.Statement) => {
getStatementSettings: (statement: ts.Statement): StatementSettings => {
if (isAmbientModule(statement) || ts.isExportDeclaration(statement)) {
return false;
return { shouldHaveExportKeyword: false, shouldHaveJSDoc: true };
}

const statementExports = getExportsForStatement(rootFileExports, typeChecker, statement);
Expand All @@ -1152,19 +1152,22 @@ export function generateDtsBundle(entries: readonly EntryPointConfig[], options:
// an export keyword (like interface, type, etc) otherwise, if there are
// only re-exports with renaming (like export { foo as bar }) we don't need
// to put export keyword for this statement because we'll re-export it in the way
const isExplicitlyExported = statementExports.find((exp: SourceFileExport) => {
const isExplicitlyExportedWithOriginalName = statementExports.find((exp: SourceFileExport) => {
if (ts.isVariableStatement(statement)) {
for (const variableDeclaration of statement.declarationList.declarations) {
if (ts.isIdentifier(variableDeclaration.name)) {
const resolvedName = collisionsResolver.resolveReferencedIdentifier(variableDeclaration.name);
if (exp.exportedName === resolvedName) {
return true;
}

continue;
}

// it seems that the compiler doesn't produce anything else (e.g. binding elements) in declaration files
// but it is still possible to write such code manually
// this feels like quite rare case so no support for now
warnLog(`Unhandled variable identifier type detected (${ts.SyntaxKind[variableDeclaration.name.kind]}). Please report this issue to https://github.com/timocov/dts-bundle-generator`);
}

return false;
Expand All @@ -1187,17 +1190,19 @@ export function generateDtsBundle(entries: readonly EntryPointConfig[], options:
if (onlyExplicitlyExportedShouldBeExported) {
// "valuable" statements must be re-exported from root source file
// to having export keyword in declaration file
return isExplicitlyExported;
return { shouldHaveExportKeyword: isExplicitlyExportedWithOriginalName, shouldHaveJSDoc: statementExports.length !== 0 };
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
if (isNodeNamedDeclaration(statement) && !isExportedWithLocalName(statement, getNodeName(statement)!.getText())) {
// if a type node was renamed because of name collisions it shouldn't be exported with its new name
renamedAndNotExplicitlyExportedTypes.push(statement);
return false;
return { shouldHaveExportKeyword: false, shouldHaveJSDoc: statementExports.length !== 0 };
}

return isExplicitlyExported || statementExports.length === 0;
// at this point a statement of a type (interface, const enum, etc) will be exported 100% (it's just a matter of a name)
// so it must have jsdoc comment
return { shouldHaveExportKeyword: isExplicitlyExportedWithOriginalName || statementExports.length === 0, shouldHaveJSDoc: true };
},
needStripConstFromConstEnum: (constEnum: ts.EnumDeclaration) => {
if (!program.getCompilerOptions().preserveConstEnums || !outputOptions.respectPreserveConstEnum) {
Expand Down
13 changes: 9 additions & 4 deletions src/generate-output.ts
Expand Up @@ -24,8 +24,13 @@ export interface NeedStripDefaultKeywordResult {
newName?: string;
}

export interface StatementSettings {
shouldHaveExportKeyword: boolean;
shouldHaveJSDoc: boolean;
}

export interface OutputHelpers {
shouldStatementHasExportKeyword(statement: ts.Statement): boolean;
getStatementSettings(statement: ts.Statement): StatementSettings;
needStripConstFromConstEnum(constEnum: ts.EnumDeclaration): boolean;
needStripImportFromImportTypeNode(importType: ts.ImportTypeNode): boolean;
resolveIdentifierName(identifier: ts.Identifier | ts.QualifiedName | ts.PropertyAccessEntityNameExpression): string | null;
Expand Down Expand Up @@ -177,7 +182,7 @@ function recreateEntityName(node: ts.EntityName, helpers: OutputHelpers): ts.Ent
}

function getStatementText(statement: ts.Statement, includeSortingValue: boolean, helpers: OutputHelpers): StatementText {
const shouldStatementHasExportKeyword = helpers.shouldStatementHasExportKeyword(statement);
const { shouldHaveExportKeyword, shouldHaveJSDoc } = helpers.getStatementSettings(statement);

// re-export statements do not contribute to top-level names scope so we don't need to resolve their identifiers
const needResolveIdentifiers = !ts.isExportDeclaration(statement) || statement.moduleSpecifier === undefined;
Expand Down Expand Up @@ -267,7 +272,7 @@ function getStatementText(statement: ts.Statement, includeSortingValue: boolean,
}
}

if (!shouldStatementHasExportKeyword) {
if (!shouldHaveExportKeyword) {
modifiersMap[ts.SyntaxKind.ExportKeyword] = false;
} else {
modifiersMap[ts.SyntaxKind.ExportKeyword] = true;
Expand All @@ -287,7 +292,7 @@ function getStatementText(statement: ts.Statement, includeSortingValue: boolean,
modifiersMap[ts.SyntaxKind.DeclareKeyword] = true;
}

return recreateRootLevelNodeWithModifiers(node, modifiersMap, resolvedStatementName, shouldStatementHasExportKeyword);
return recreateRootLevelNodeWithModifiers(node, modifiersMap, resolvedStatementName, shouldHaveJSDoc);
},
}
);
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/test-cases/names-collision-across-files/file2.ts
Expand Up @@ -17,10 +17,12 @@ export function func(two: number) {}

export type TypeName = Pick<Interface, 'field2'>;

/** Another interface doc string */
export interface AnotherInterface {
field2: number;
}

/** Another func doc string */
export function anotherFunc(two: NamespaceName.Local) {}

export namespace NamespaceName {
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/test-cases/names-collision-across-files/output.d.ts
Expand Up @@ -53,9 +53,11 @@ interface Interface$1 {
}
declare function func$1(two: number): void;
type TypeName$1 = Pick<Interface$1, "field2">;
/** Another interface doc string */
interface AnotherInterface$1 {
field2: number;
}
/** Another func doc string */
declare function anotherFunc$1(two: NamespaceName$1.Local): void;
declare namespace NamespaceName$1 {
interface Local {
Expand Down

0 comments on commit 2b1b281

Please sign in to comment.