Skip to content

[api-extractor] Add handling of file reference directives #1955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
willmtemple wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[api-extractor] Add handling of file reference directives #1955

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e16800e
[api-extractor] Add handling of file reference directives
willmtemple Jun 22, 2020
b9969ed
Add test, use path primitives
willmtemple Jun 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions apps/api-extractor/src/collector/Collector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,29 @@ export interface ICollectorOptions {
extractorConfig: ExtractorConfig;
}

/**
* Resolve the name of a file to an absolute path relative to a TypeScript SourceFile node.
*/
function resolveNameRelativeToSourceFile(name: string, source: ts.SourceFile): string {

@octogonz octogonz Jun 23, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you simply use path.relative()?

@willmtemple willmtemple Jun 23, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one, I don't think so. I need to boot into Windows and check it, but I believe that the reference directive in text has to follow the POSIX convention and use forward slash as its path separator just like a node module, but the file name of the source file could be in Windows convention, so relying on path to process the reference path may not be safe. Need to do some testing.

@octogonz octogonz Jun 26, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the input and output paths are POSIX format, then you can use path.posix.relative().

If the input path is Windows style and you want to convert to POSIX, there is no theoretically correct API for that, but Text.replaceAll(thePath, '\\', '/') works reasonably well with Node.js

const output: string[] = source.fileName.split('/').slice(0, -1);

// Process the file name, treating special selectors such as '..' and '.' and '' correctly
for (const fragment of name.split('/')) {
switch (fragment) {
case '..': // parent selector
output.pop();
break;
case '.': // same-dir selectors
case '':
break;
default:
output.push(fragment);
}
}

return output.join('/');
}

/**
* The `Collector` manages the overall data set that is used by `ApiModelGenerator`,
* `DtsRollupGenerator`, and `ApiReportGenerator`. Starting from the working package's entry point,
Expand Down Expand Up @@ -84,6 +107,7 @@ export class Collector {

private readonly _dtsTypeReferenceDirectives: Set<string> = new Set<string>();
private readonly _dtsLibReferenceDirectives: Set<string> = new Set<string>();
private readonly _dtsFileReferenceDirectives: Set<string> = new Set<string>();

// Used by getOverloadIndex()
private readonly _cachedOverloadIndexesByDeclaration: Map<AstDeclaration, number>;
Expand Down Expand Up @@ -158,6 +182,17 @@ export class Collector {
return this._dtsLibReferenceDirectives;
}

/**
* A list of names (e.g. "runtime-library") that should appear in a path-based reference like this:
*
* ```
* /// <reference path="runtime-library" />
* ```
*/
public get dtsFileReferenceDirectives(): ReadonlySet<string> {

@octogonz octogonz Jun 23, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime-library example is not intended to be a path. Maybe something like this:

  /**
   * A list of paths (e.g. "path/to/file.d.ts") that should appear in a reference like this:
   *
   * ```
   * /// <reference path="path/to/file.d.ts" />
   * ```
   *
   * The paths are resolved relative to the source file containing the reference.
   */

@willmtemple willmtemple Jun 23, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah just missed this when copying the doc string 👍

return this._dtsFileReferenceDirectives;
}

public get entities(): ReadonlyArray<CollectorEntity> {
return this._entities;
}
Expand Down Expand Up @@ -262,6 +297,7 @@ export class Collector {
Sort.sortBy(this._entities, (x) => x.getSortKey());
Sort.sortSet(this._dtsTypeReferenceDirectives);
Sort.sortSet(this._dtsLibReferenceDirectives);
Sort.sortSet(this._dtsFileReferenceDirectives);
this._starExportedExternalModulePaths.sort();
}

Expand Down Expand Up @@ -875,6 +911,11 @@ export class Collector {
);
this._dtsLibReferenceDirectives.add(name);
}

for (const referencedFile of sourceFile.referencedFiles) {
const name: string = sourceFile.text.substring(referencedFile.pos, referencedFile.end);
this._dtsFileReferenceDirectives.add(resolveNameRelativeToSourceFile(name, sourceFile));
}
}
}
}
Expand Down
37 changes: 36 additions & 1 deletion apps/api-extractor/src/generators/DtsRollupGenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,35 @@ export enum DtsRollupKind {
PublicRelease
}

/**
* Compute a relative path between two absolute paths.
*/
function computeRelativePath(fromPath: string, toPath: string): string {

@octogonz octogonz Jun 23, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you simply use path.relative()?

@willmtemple willmtemple Jun 23, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this instance, sure. The Path module in the node-core-library package in this repo made me think that there was some reason for avoiding it.

@octogonz octogonz Jun 26, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileSystem is a complete replacement for fs, because the native fs library was designed as low-level primitives that are not suitable for everyday usage.

Path contains supplementary stuff that merely extends the native path, since that API is fine to use.

We should probably document this better.

const fromPathComponents: string[] = fromPath.split('/').reverse();
const toPathComponents: string[] = toPath.split('/').reverse();

// Remove all common parts of the paths
while (
fromPathComponents.length > 0 &&
toPathComponents.length > 0 &&
fromPathComponents[fromPathComponents.length - 1] === toPathComponents[toPathComponents.length - 1]
) {
fromPathComponents.length -= 1;
toPathComponents.length -= 1;
}

// The final relative path consists of enough parent selectors to move up
// from the remaining "from path" directory components joined to the
// remaining "to path" components
return (
Array(fromPathComponents.length - 1)
.fill('..')
.join('/') +
'/' +
toPathComponents.join('/')
);
}

export class DtsRollupGenerator {
/**
* Generates the typings file and writes it to disk.
Expand All @@ -59,7 +88,7 @@ export class DtsRollupGenerator {
): void {
const stringWriter: StringWriter = new StringWriter();

DtsRollupGenerator._generateTypingsFileContent(collector, stringWriter, dtsKind);
DtsRollupGenerator._generateTypingsFileContent(collector, dtsFilename, stringWriter, dtsKind);

FileSystem.writeFile(dtsFilename, stringWriter.toString(), {
convertLineEndings: newlineKind,
Expand All @@ -69,6 +98,7 @@ export class DtsRollupGenerator {

private static _generateTypingsFileContent(
collector: Collector,
dtsFilename: string,
stringWriter: StringWriter,
dtsKind: DtsRollupKind
): void {
Expand All @@ -87,6 +117,11 @@ export class DtsRollupGenerator {
stringWriter.writeLine(`/// <reference lib="${libDirectiveReference}" />`);
}

for (const fileDirectiveReference of collector.dtsFileReferenceDirectives) {
const correctedRelativePath: string = computeRelativePath(dtsFilename, fileDirectiveReference);
stringWriter.writeLine(`/// <reference path="${correctedRelativePath}" />`);
}

// Emit the imports
for (const entity of collector.entities) {
if (entity.astEntity instanceof AstImport) {
Expand Down