Skip to content

Commit 9a25abc

Browse files
iclantoniclanton
authored
[api-extractor] Fix an issue with preservation of triple-slash references. (#5131)
* Fix an issue with preservation of triple-slash references. * fixup! Fix an issue with preservation of triple-slash references. * fixup! Fix an issue with preservation of triple-slash references.
1 parent 33db957 commit 9a25abc

File tree

19 files changed

+153
-60
lines changed

19 files changed

+153
-60
lines changed

‎apps/api-extractor/src/analyzer/AstModule.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ import type { AstEntity } from './AstEntity';
99
/**
1010
* Represents information collected by {@link AstSymbolTable.fetchAstModuleExportInfo}
1111
*/
12-
export class AstModuleExportInfo {
13-
public readonly exportedLocalEntities: Map<string, AstEntity> = new Map<string, AstEntity>();
14-
public readonly starExportedExternalModules: Set<AstModule> = new Set<AstModule>();
12+
export interface IAstModuleExportInfo {
13+
readonly visitedAstModules: Set<AstModule>;
14+
readonly exportedLocalEntities: Map<string, AstEntity>;
15+
readonly starExportedExternalModules: Set<AstModule>;
1516
}
1617

1718
/**
@@ -64,7 +65,7 @@ export class AstModule {
6465
/**
6566
* Additional state calculated by `AstSymbolTable.fetchWorkingPackageModule()`.
6667
*/
67-
public astModuleExportInfo: AstModuleExportInfo | undefined;
68+
public astModuleExportInfo: IAstModuleExportInfo | undefined;
6869

6970
public constructor(options: IAstModuleOptions) {
7071
this.sourceFile = options.sourceFile;

‎apps/api-extractor/src/analyzer/AstNamespaceImport.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import type * as ts from 'typescript';
55

6-
import type { AstModule, AstModuleExportInfo } from './AstModule';
6+
import type { AstModule, IAstModuleExportInfo } from './AstModule';
77
import { AstSyntheticEntity } from './AstEntity';
88
import type { Collector } from '../collector/Collector';
99

@@ -87,8 +87,8 @@ export class AstNamespaceImport extends AstSyntheticEntity {
8787
return this.namespaceName;
8888
}
8989

90-
public fetchAstModuleExportInfo(collector: Collector): AstModuleExportInfo {
91-
const astModuleExportInfo: AstModuleExportInfo = collector.astSymbolTable.fetchAstModuleExportInfo(
90+
public fetchAstModuleExportInfo(collector: Collector): IAstModuleExportInfo {
91+
const astModuleExportInfo: IAstModuleExportInfo = collector.astSymbolTable.fetchAstModuleExportInfo(
9292
this.astModule
9393
);
9494
return astModuleExportInfo;

‎apps/api-extractor/src/analyzer/AstSymbolTable.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { type PackageJsonLookup, InternalError } from '@rushstack/node-core-libr
99
import { AstDeclaration } from './AstDeclaration';
1010
import { TypeScriptHelpers } from './TypeScriptHelpers';
1111
import { AstSymbol } from './AstSymbol';
12-
import type { AstModule, AstModuleExportInfo } from './AstModule';
12+
import type { AstModule, IAstModuleExportInfo } from './AstModule';
1313
import { PackageMetadataManager } from './PackageMetadataManager';
1414
import { ExportAnalyzer } from './ExportAnalyzer';
1515
import type { AstEntity } from './AstEntity';
@@ -124,7 +124,7 @@ export class AstSymbolTable {
124124
/**
125125
* This crawls the specified entry point and collects the full set of exported AstSymbols.
126126
*/
127-
public fetchAstModuleExportInfo(astModule: AstModule): AstModuleExportInfo {
127+
public fetchAstModuleExportInfo(astModule: AstModule): IAstModuleExportInfo {
128128
return this._exportAnalyzer.fetchAstModuleExportInfo(astModule);
129129
}
130130

‎apps/api-extractor/src/analyzer/ExportAnalyzer.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { InternalError } from '@rushstack/node-core-library';
77
import { TypeScriptHelpers } from './TypeScriptHelpers';
88
import { AstSymbol } from './AstSymbol';
99
import { AstImport, type IAstImportOptions, AstImportKind } from './AstImport';
10-
import { AstModule, AstModuleExportInfo } from './AstModule';
10+
import { AstModule, type IAstModuleExportInfo } from './AstModule';
1111
import { TypeScriptInternals } from './TypeScriptInternals';
1212
import { SourceFileLocationFormatter } from './SourceFileLocationFormatter';
1313
import type { IFetchAstSymbolOptions } from './AstSymbolTable';
@@ -237,15 +237,19 @@ export class ExportAnalyzer {
237237
/**
238238
* Implementation of {@link AstSymbolTable.fetchAstModuleExportInfo}.
239239
*/
240-
public fetchAstModuleExportInfo(entryPointAstModule: AstModule): AstModuleExportInfo {
240+
public fetchAstModuleExportInfo(entryPointAstModule: AstModule): IAstModuleExportInfo {
241241
if (entryPointAstModule.isExternal) {
242242
throw new Error('fetchAstModuleExportInfo() is not supported for external modules');
243243
}
244244

245245
if (entryPointAstModule.astModuleExportInfo === undefined) {
246-
const astModuleExportInfo: AstModuleExportInfo = new AstModuleExportInfo();
246+
const astModuleExportInfo: IAstModuleExportInfo = {
247+
visitedAstModules: new Set<AstModule>(),
248+
exportedLocalEntities: new Map<string, AstEntity>(),
249+
starExportedExternalModules: new Set<AstModule>()
250+
};
247251

248-
this._collectAllExportsRecursive(astModuleExportInfo, entryPointAstModule, new Set<AstModule>());
252+
this._collectAllExportsRecursive(astModuleExportInfo, entryPointAstModule);
249253

250254
entryPointAstModule.astModuleExportInfo = astModuleExportInfo;
251255
}
@@ -314,18 +318,15 @@ export class ExportAnalyzer {
314318
return this._importableAmbientSourceFiles.has(sourceFile);
315319
}
316320

317-
private _collectAllExportsRecursive(
318-
astModuleExportInfo: AstModuleExportInfo,
319-
astModule: AstModule,
320-
visitedAstModules: Set<AstModule>
321-
): void {
321+
private _collectAllExportsRecursive(astModuleExportInfo: IAstModuleExportInfo, astModule: AstModule): void {
322+
const { visitedAstModules, starExportedExternalModules, exportedLocalEntities } = astModuleExportInfo;
322323
if (visitedAstModules.has(astModule)) {
323324
return;
324325
}
325326
visitedAstModules.add(astModule);
326327

327328
if (astModule.isExternal) {
328-
astModuleExportInfo.starExportedExternalModules.add(astModule);
329+
starExportedExternalModules.add(astModule);
329330
} else {
330331
// Fetch each of the explicit exports for this module
331332
if (astModule.moduleSymbol.exports) {
@@ -337,7 +338,7 @@ export class ExportAnalyzer {
337338
default:
338339
// Don't collect the "export default" symbol unless this is the entry point module
339340
if (exportName !== ts.InternalSymbolName.Default || visitedAstModules.size === 1) {
340-
if (!astModuleExportInfo.exportedLocalEntities.has(exportSymbol.name)) {
341+
if (!exportedLocalEntities.has(exportSymbol.name)) {
341342
const astEntity: AstEntity = this._getExportOfAstModule(exportSymbol.name, astModule);
342343

343344
if (astEntity instanceof AstSymbol && !astEntity.isExternal) {
@@ -348,7 +349,7 @@ export class ExportAnalyzer {
348349
this._astSymbolTable.analyze(astEntity);
349350
}
350351

351-
astModuleExportInfo.exportedLocalEntities.set(exportSymbol.name, astEntity);
352+
exportedLocalEntities.set(exportSymbol.name, astEntity);
352353
}
353354
}
354355
break;
@@ -357,7 +358,7 @@ export class ExportAnalyzer {
357358
}
358359

359360
for (const starExportedModule of astModule.starExportedModules) {
360-
this._collectAllExportsRecursive(astModuleExportInfo, starExportedModule, visitedAstModules);
361+
this._collectAllExportsRecursive(astModuleExportInfo, starExportedModule);
361362
}
362363
}
363364
}

‎apps/api-extractor/src/collector/Collector.ts‎

Lines changed: 84 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { ExtractorMessageId } from '../api/ExtractorMessageId';
1818
import { CollectorEntity } from './CollectorEntity';
1919
import { AstSymbolTable } from '../analyzer/AstSymbolTable';
2020
import type { AstEntity } from '../analyzer/AstEntity';
21-
import type { AstModule, AstModuleExportInfo } from '../analyzer/AstModule';
21+
import type { AstModule, IAstModuleExportInfo } from '../analyzer/AstModule';
2222
import { AstSymbol } from '../analyzer/AstSymbol';
2323
import type { AstDeclaration } from '../analyzer/AstDeclaration';
2424
import { TypeScriptHelpers } from '../analyzer/TypeScriptHelpers';
@@ -313,12 +313,12 @@ export class Collector {
313313
this.workingPackage.tsdocComment = this.workingPackage.tsdocParserContext!.docComment;
314314
}
315315

316-
const astModuleExportInfo: AstModuleExportInfo =
316+
const { exportedLocalEntities, starExportedExternalModules, visitedAstModules }: IAstModuleExportInfo =
317317
this.astSymbolTable.fetchAstModuleExportInfo(astEntryPoint);
318318

319319
// Create a CollectorEntity for each top-level export.
320320
const processedAstEntities: AstEntity[] = [];
321-
for (const [exportName, astEntity] of astModuleExportInfo.exportedLocalEntities) {
321+
for (const [exportName, astEntity] of exportedLocalEntities) {
322322
this._createCollectorEntity(astEntity, exportName);
323323
processedAstEntities.push(astEntity);
324324
}
@@ -333,9 +333,33 @@ export class Collector {
333333
}
334334
}
335335

336+
// Ensure references are collected from any intermediate files that
337+
// only include exports
338+
const nonExternalSourceFiles: Set<ts.SourceFile> = new Set();
339+
for (const { sourceFile, isExternal } of visitedAstModules) {
340+
if (!nonExternalSourceFiles.has(sourceFile) && !isExternal) {
341+
nonExternalSourceFiles.add(sourceFile);
342+
}
343+
}
344+
345+
// Here, we're collecting reference directives from all non-external source files
346+
// that were encountered while looking for exports, but only those references that
347+
// were explicitly written by the developer and marked with the `preserve="true"`
348+
// attribute. In TS >= 5.5, only references that are explicitly authored and marked
349+
// with `preserve="true"` are included in the output. See https://github.com/microsoft/TypeScript/pull/57681
350+
//
351+
// The `_collectReferenceDirectives` function pulls in all references in files that
352+
// contain definitions, but does not examine files that only reexport from other
353+
// files. Here, we're looking through files that were missed by `_collectReferenceDirectives`,
354+
// but only collecting references that were explicitly marked with `preserve="true"`.
355+
// It is intuitive for developers to include references that they explicitly want part of
356+
// their public API in a file like the entrypoint, which is likely to only contain reexports,
357+
// and this picks those up.
358+
this._collectReferenceDirectivesFromSourceFiles(nonExternalSourceFiles, true);
359+
336360
this._makeUniqueNames();
337361

338-
for (const starExportedExternalModule of astModuleExportInfo.starExportedExternalModules) {
362+
for (const starExportedExternalModule of starExportedExternalModules) {
339363
if (starExportedExternalModule.externalModulePath !== undefined) {
340364
this._starExportedExternalModulePaths.push(starExportedExternalModule.externalModulePath);
341365
}
@@ -539,7 +563,7 @@ export class Collector {
539563
}
540564

541565
if (astEntity instanceof AstNamespaceImport) {
542-
const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(this);
566+
const astModuleExportInfo: IAstModuleExportInfo = astEntity.fetchAstModuleExportInfo(this);
543567
const parentEntity: CollectorEntity | undefined = this._entitiesByAstEntity.get(astEntity);
544568
if (!parentEntity) {
545569
// This should never happen, as we've already created entities for all AstNamespaceImports.
@@ -992,44 +1016,82 @@ export class Collector {
9921016
}
9931017

9941018
private _collectReferenceDirectives(astEntity: AstEntity): void {
1019+
// Here, we're collecting reference directives from source files that contain extracted
1020+
// definitions (i.e. - files that contain `export class ...`, `export interface ...`, ...).
1021+
// These references may or may not include the `preserve="true" attribute. In TS < 5.5,
1022+
// references that end up in .D.TS files may or may not be explicity written by the developer.
1023+
// In TS >= 5.5, only references that are explicitly authored and are marked with
1024+
// `preserve="true"` are included in the output. See https://github.com/microsoft/TypeScript/pull/57681
1025+
//
1026+
// The calls to `_collectReferenceDirectivesFromSourceFiles` in this function are
1027+
// preserving existing behavior, which is to include all reference directives
1028+
// regardless of whether they are explicitly authored or not, but only in files that
1029+
// contain definitions.
1030+
9951031
if (astEntity instanceof AstSymbol) {
9961032
const sourceFiles: ts.SourceFile[] = astEntity.astDeclarations.map((astDeclaration) =>
9971033
astDeclaration.declaration.getSourceFile()
9981034
);
999-
return this._collectReferenceDirectivesFromSourceFiles(sourceFiles);
1035+
return this._collectReferenceDirectivesFromSourceFiles(sourceFiles, false);
10001036
}
10011037

10021038
if (astEntity instanceof AstNamespaceImport) {
10031039
const sourceFiles: ts.SourceFile[] = [astEntity.astModule.sourceFile];
1004-
return this._collectReferenceDirectivesFromSourceFiles(sourceFiles);
1040+
return this._collectReferenceDirectivesFromSourceFiles(sourceFiles, false);
10051041
}
10061042
}
10071043

1008-
private _collectReferenceDirectivesFromSourceFiles(sourceFiles: ts.SourceFile[]): void {
1044+
private _collectReferenceDirectivesFromSourceFiles(
1045+
sourceFiles: Iterable<ts.SourceFile>,
1046+
onlyIncludeExplicitlyPreserved: boolean
1047+
): void {
10091048
const seenFilenames: Set<string> = new Set<string>();
10101049

10111050
for (const sourceFile of sourceFiles) {
1012-
if (sourceFile && sourceFile.fileName) {
1013-
if (!seenFilenames.has(sourceFile.fileName)) {
1014-
seenFilenames.add(sourceFile.fileName);
1015-
1016-
for (const typeReferenceDirective of sourceFile.typeReferenceDirectives) {
1017-
const name: string = sourceFile.text.substring(
1018-
typeReferenceDirective.pos,
1019-
typeReferenceDirective.end
1051+
if (sourceFile?.fileName) {
1052+
const {
1053+
fileName,
1054+
typeReferenceDirectives,
1055+
libReferenceDirectives,
1056+
text: sourceFileText
1057+
} = sourceFile;
1058+
if (!seenFilenames.has(fileName)) {
1059+
seenFilenames.add(fileName);
1060+
1061+
for (const typeReferenceDirective of typeReferenceDirectives) {
1062+
const name: string | undefined = this._getReferenceDirectiveFromSourceFile(
1063+
sourceFileText,
1064+
typeReferenceDirective,
1065+
onlyIncludeExplicitlyPreserved
10201066
);
1021-
this._dtsTypeReferenceDirectives.add(name);
1067+
if (name) {
1068+
this._dtsTypeReferenceDirectives.add(name);
1069+
}
10221070
}
10231071

1024-
for (const libReferenceDirective of sourceFile.libReferenceDirectives) {
1025-
const name: string = sourceFile.text.substring(
1026-
libReferenceDirective.pos,
1027-
libReferenceDirective.end
1072+
for (const libReferenceDirective of libReferenceDirectives) {
1073+
const reference: string | undefined = this._getReferenceDirectiveFromSourceFile(
1074+
sourceFileText,
1075+
libReferenceDirective,
1076+
onlyIncludeExplicitlyPreserved
10281077
);
1029-
this._dtsLibReferenceDirectives.add(name);
1078+
if (reference) {
1079+
this._dtsLibReferenceDirectives.add(reference);
1080+
}
10301081
}
10311082
}
10321083
}
10331084
}
10341085
}
1086+
1087+
private _getReferenceDirectiveFromSourceFile(
1088+
sourceFileText: string,
1089+
{ pos, end, preserve }: ts.FileReference,
1090+
onlyIncludeExplicitlyPreserved: boolean
1091+
): string | undefined {
1092+
const reference: string = sourceFileText.substring(pos, end);
1093+
if (preserve || !onlyIncludeExplicitlyPreserved) {
1094+
return reference;
1095+
}
1096+
}
10351097
}

‎apps/api-extractor/src/enhancers/ValidationEnhancer.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type { CollectorEntity } from '../collector/CollectorEntity';
1313
import { ExtractorMessageId } from '../api/ExtractorMessageId';
1414
import { ReleaseTag } from '@microsoft/api-extractor-model';
1515
import { AstNamespaceImport } from '../analyzer/AstNamespaceImport';
16-
import type { AstModuleExportInfo } from '../analyzer/AstModule';
16+
import type { IAstModuleExportInfo } from '../analyzer/AstModule';
1717
import type { AstEntity } from '../analyzer/AstEntity';
1818

1919
export class ValidationEnhancer {
@@ -47,7 +47,7 @@ export class ValidationEnhancer {
4747
// A namespace created using "import * as ___ from ___"
4848
const astNamespaceImport: AstNamespaceImport = entity.astEntity;
4949

50-
const astModuleExportInfo: AstModuleExportInfo =
50+
const astModuleExportInfo: IAstModuleExportInfo =
5151
astNamespaceImport.fetchAstModuleExportInfo(collector);
5252

5353
for (const namespaceMemberAstEntity of astModuleExportInfo.exportedLocalEntities.values()) {

‎apps/api-extractor/src/generators/ApiReportGenerator.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { IndentedWriter } from './IndentedWriter';
1818
import { DtsEmitHelpers } from './DtsEmitHelpers';
1919
import { AstNamespaceImport } from '../analyzer/AstNamespaceImport';
2020
import type { AstEntity } from '../analyzer/AstEntity';
21-
import type { AstModuleExportInfo } from '../analyzer/AstModule';
21+
import type { IAstModuleExportInfo } from '../analyzer/AstModule';
2222
import { SourceFileLocationFormatter } from '../analyzer/SourceFileLocationFormatter';
2323
import { ExtractorMessageId } from '../api/ExtractorMessageId';
2424
import type { ApiReportVariant } from '../api/IConfigFile';
@@ -153,7 +153,7 @@ export class ApiReportGenerator {
153153
}
154154

155155
if (astEntity instanceof AstNamespaceImport) {
156-
const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
156+
const astModuleExportInfo: IAstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
157157

158158
if (entity.nameForEmit === undefined) {
159159
// This should never happen

‎apps/api-extractor/src/generators/DtsRollupGenerator.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { IndentedWriter } from './IndentedWriter';
2020
import { DtsEmitHelpers } from './DtsEmitHelpers';
2121
import type { DeclarationMetadata } from '../collector/DeclarationMetadata';
2222
import { AstNamespaceImport } from '../analyzer/AstNamespaceImport';
23-
import type { AstModuleExportInfo } from '../analyzer/AstModule';
23+
import type { IAstModuleExportInfo } from '../analyzer/AstModule';
2424
import { SourceFileLocationFormatter } from '../analyzer/SourceFileLocationFormatter';
2525
import type { AstEntity } from '../analyzer/AstEntity';
2626

@@ -153,7 +153,7 @@ export class DtsRollupGenerator {
153153
}
154154

155155
if (astEntity instanceof AstNamespaceImport) {
156-
const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
156+
const astModuleExportInfo: IAstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
157157

158158
if (entity.nameForEmit === undefined) {
159159
// This should never happen

‎build-tests/api-extractor-test-02/dist/api-extractor-test-02-alpha.d.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
* @packageDocumentation
88
*/
99

10+
/// <reference types="long" />
11+
1012
import { ISimpleInterface } from 'api-extractor-test-01';
1113
import { ReexportedClass as RenamedReexportedClass3 } from 'api-extractor-test-01';
1214
import * as semver1 from 'semver';

‎build-tests/api-extractor-test-02/dist/api-extractor-test-02-beta.d.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
* @packageDocumentation
88
*/
99

10+
/// <reference types="long" />
11+
1012
import { ISimpleInterface } from 'api-extractor-test-01';
1113
import { ReexportedClass as RenamedReexportedClass3 } from 'api-extractor-test-01';
1214
import * as semver1 from 'semver';

0 commit comments

Comments
 (0)