Skip to content

Commit

Permalink
feat(ng-dev/ts-circular-dependencies): support ignoring type only imp…
Browse files Browse the repository at this point in the history
…orts/exports for circular dependency checks (#772)

By setting a configuration, imports/exports which use the `type` keyword to indicate they are type-only imports/exports
are ignored with respect to circular dependency checks.

PR Close #772
  • Loading branch information
josephperrott committed Aug 17, 2022
1 parent f502be3 commit 682adb7
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 12 deletions.
10 changes: 8 additions & 2 deletions ng-dev/ts-circular-dependencies/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import {readFileSync} from 'fs';
import {dirname, join, resolve} from 'path';
import ts from 'typescript';
import {CircularDependenciesParserOptions} from './config.js';

import {getFileStatus} from './file_system.js';
import {getModuleReferences} from './parser.js';
Expand All @@ -32,13 +33,18 @@ const DEFAULT_EXTENSIONS = ['ts', 'js', 'd.ts'];
export class Analyzer {
private _sourceFileCache = new Map<string, ts.SourceFile>();

private _ignoreTypeOnlyChecks: boolean;

unresolvedModules = new Set<string>();
unresolvedFiles = new Map<string, string[]>();

constructor(
public resolveModuleFn?: ModuleResolver,
ignoreTypeOnlyChecks: boolean = false,
public extensions: string[] = DEFAULT_EXTENSIONS,
) {}
) {
this._ignoreTypeOnlyChecks = !!ignoreTypeOnlyChecks;
}

/** Finds all cycles in the specified source file. */
findCycles(
Expand All @@ -61,7 +67,7 @@ export class Analyzer {
visited.add(sf);
// Go through all edges, which are determined through import/exports, and collect cycles.
const result: ReferenceChain[] = [];
for (const ref of getModuleReferences(sf)) {
for (const ref of getModuleReferences(sf, this._ignoreTypeOnlyChecks)) {
const targetFile = this._resolveImport(ref, sf.fileName);
if (targetFile !== null) {
result.push(...this.findCycles(this.getSourceFile(targetFile), visited, path.slice()));
Expand Down
8 changes: 7 additions & 1 deletion ng-dev/ts-circular-dependencies/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ import {Log} from '../utils/logging.js';

import {ModuleResolver} from './analyzer.js';

/** Options used at runtime by the parser. */
export interface CircularDependenciesParserOptions {
/** Whether to ignore type only imports in circular dependency checks. */
ignoreTypeOnlyChecks?: true;
}

/** Configuration for a circular dependencies test. */
export interface CircularDependenciesTestConfig {
export interface CircularDependenciesTestConfig extends CircularDependenciesParserOptions {
/** Base directory used for shortening paths in the golden file. */
baseDir: string;
/** Path to the golden file that is used for checking and approving. */
Expand Down
11 changes: 9 additions & 2 deletions ng-dev/ts-circular-dependencies/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,15 @@ export function main(
config: CircularDependenciesTestConfig,
printWarnings: boolean,
): number {
const {baseDir, goldenFile, glob: globPattern, resolveModule, approveCommand} = config;
const analyzer = new Analyzer(resolveModule);
const {
baseDir,
goldenFile,
glob: globPattern,
resolveModule,
approveCommand,
ignoreTypeOnlyChecks,
} = config;
const analyzer = new Analyzer(resolveModule, ignoreTypeOnlyChecks);
const cycles: ReferenceChain[] = [];
const checkedNodes = new WeakSet<ts.SourceFile>();

Expand Down
24 changes: 17 additions & 7 deletions ng-dev/ts-circular-dependencies/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,25 @@ import ts from 'typescript';
* @param node Source file which should be parsed.
* @returns List of import specifiers in the source file.
*/
export function getModuleReferences(initialNode: ts.SourceFile): string[] {
export function getModuleReferences(
initialNode: ts.SourceFile,
ignoreTypeOnlyChecks: boolean,
): string[] {
const references: string[] = [];
const visitNode = (node: ts.Node) => {
if (
(ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) &&
node.moduleSpecifier !== undefined &&
ts.isStringLiteral(node.moduleSpecifier)
) {
references.push(node.moduleSpecifier.text);
if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) {
// When ignoreTypeOnlyChecks are enabled, if the declaration is found to be type only, it is skipped.
if (
ignoreTypeOnlyChecks &&
((ts.isImportDeclaration(node) && node.importClause?.isTypeOnly) ||
(ts.isExportDeclaration(node) && node.isTypeOnly))
) {
return;
}

if (node.moduleSpecifier !== undefined && ts.isStringLiteral(node.moduleSpecifier)) {
references.push(node.moduleSpecifier.text);
}
}
ts.forEachChild(node, visitNode);
};
Expand Down

0 comments on commit 682adb7

Please sign in to comment.