Skip to content

Commit

Permalink
Support non-entry cross-reference imports in workspaces (resolves #244)
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Sep 24, 2023
1 parent 16f16ae commit 8ab8992
Show file tree
Hide file tree
Showing 16 changed files with 163 additions and 57 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions fixtures/workspaces-cross-reference/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@workspaces-cross-reference/root",
"workspaces": [
"packages/*"
]
}
2 changes: 2 additions & 0 deletions fixtures/workspaces-cross-reference/packages/lib-a/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import a from './mod-a';
a;
7 changes: 7 additions & 0 deletions fixtures/workspaces-cross-reference/packages/lib-a/mod-a.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import circularA, { B1 } from '@workspaces-cross-reference/lib-b/mod-b';

export const A = B1;

export const unused = true;

export default circularA;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@workspaces-cross-reference/lib-a",
"dependencies": {
"@workspaces-cross-reference/lib-b": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"baseUrl": "./"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 1;
7 changes: 7 additions & 0 deletions fixtures/workspaces-cross-reference/packages/lib-b/mod-b.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { A } from '@workspaces-cross-reference/lib-a/mod-a';

export const B1 = 'B1';

export const unused = true;

export default A;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@workspaces-cross-reference/lib-b",
"dependencies": {
"@workspaces-cross-reference/lib-a": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"baseUrl": "./"
}
}
4 changes: 2 additions & 2 deletions src/ConfigurationChief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type ConfigurationManagerOptions = {

export type Workspace = {
name: string;
pkgName?: string;
pkgName: string;
dir: string;
ancestors: string[];
config: WorkspaceConfiguration;
Expand Down Expand Up @@ -316,7 +316,7 @@ export class ConfigurationChief {
return workspaceNames.sort(byPathDepth).map(
(name): Workspace => ({
name,
pkgName: this.manifestWorkspaces.get(name) ?? this.manifest?.name,
pkgName: this.manifestWorkspaces.get(name) ?? this.manifest?.name ?? `NOT_FOUND_${name}`,
dir: join(this.cwd, name),
config: this.getConfigForWorkspace(name),
ancestors: this.availableWorkspaceNames.reduce(getAncestors(name), []),
Expand Down
20 changes: 13 additions & 7 deletions src/PrincipalFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import type { SyncCompilers, AsyncCompilers } from './types/compilers.js';

type Paths = ts.CompilerOptions['paths'];

type Principal = { principal: ProjectPrincipal; cwds: Set<string>; pathKeys: Set<string> };
type Principal = { principal: ProjectPrincipal; cwds: Set<string>; pathKeys: Set<string>; pkgNames: Set<string> };
type Principals = Set<Principal>;

type Options = {
cwd: string;
compilerOptions: ts.CompilerOptions;
paths: Paths;
compilers: [SyncCompilers, AsyncCompilers];
pkgName: string;
};

const mergePaths = (cwd: string, compilerOptions: ts.CompilerOptions, paths: Paths = {}) => {
Expand All @@ -33,11 +34,11 @@ export class PrincipalFactory {
principals: Principals = new Set();

public getPrincipal(options: Options) {
const { cwd, compilerOptions, paths } = options;
const { cwd, compilerOptions, paths, pkgName } = options;
options.compilerOptions = mergePaths(cwd, compilerOptions, paths);
const principal = this.findReusablePrincipal(compilerOptions);
if (principal) {
this.linkPrincipal(principal, cwd, compilerOptions);
this.linkPrincipal(principal, cwd, compilerOptions, pkgName);
return principal.principal;
} else {
return this.addNewPrincipal(options);
Expand All @@ -59,23 +60,28 @@ export class PrincipalFactory {
return principal;
}

private linkPrincipal(principal: Principal, cwd: string, compilerOptions: ts.CompilerOptions) {
private linkPrincipal(principal: Principal, cwd: string, compilerOptions: ts.CompilerOptions, pkgName: string) {
const { pathsBasePath, paths } = compilerOptions;
if (pathsBasePath) principal.principal.compilerOptions.pathsBasePath = pathsBasePath;
Object.keys(paths ?? {}).forEach(p => principal.pathKeys.add(p));
principal.principal.compilerOptions.paths = { ...principal.principal.compilerOptions.paths, ...paths };
principal.cwds.add(cwd);
principal.pkgNames.add(pkgName);
}

private addNewPrincipal(options: Options) {
const { cwd, compilerOptions } = options;
const { cwd, compilerOptions, pkgName } = options;
const pathKeys = new Set(Object.keys(compilerOptions?.paths ?? {}));
const principal = new ProjectPrincipal(options);
this.principals.add({ principal, cwds: new Set([cwd]), pathKeys });
this.principals.add({ principal, cwds: new Set([cwd]), pathKeys, pkgNames: new Set([pkgName]) });
return principal;
}

public getPrincipals() {
return Array.from(this.principals, p => p.principal);
return Array.from(this.principals, p => p.principal).reverse();
}

public getPrincipalByPackageName(packageName: string) {
return Array.from(this.principals).find(principal => principal.pkgNames.has(packageName))?.principal;
}
}
10 changes: 8 additions & 2 deletions src/ProjectPrincipal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ export class ProjectPrincipal {
}

public analyzeSourceFile(filePath: string, { skipTypeOnly }: { skipTypeOnly: boolean }) {
const sourceFile = this.backend.program?.getSourceFile(filePath);
// We request it from `fileManager` directly as `program` does not contain cross-referenced files
const sourceFile = this.backend.fileManager.getSourceFile(filePath);

if (!sourceFile) throw new Error(`Unable to find ${filePath}`);

Expand Down Expand Up @@ -270,6 +271,11 @@ export class ProjectPrincipal {
}

private findReferences(filePath: string, pos: number) {
return this.backend.lsFindReferences(filePath, pos) ?? [];
try {
return this.backend.lsFindReferences(filePath, pos) ?? [];
} catch {
// TS throws for (cross-referenced) files not in the program
return [];
}
}
}
114 changes: 68 additions & 46 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
};

for (const workspace of workspaces) {
const { name, dir, config, ancestors } = workspace;
const { name, dir, config, ancestors, pkgName } = workspace;
const { paths, ignoreDependencies, ignoreBinaries } = config;

const isRoot = name === ROOT_WORKSPACE_NAME;
Expand All @@ -142,7 +142,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

const compilerOptions = await loadCompilerOptions(join(dir, tsConfigFile ?? 'tsconfig.json'));

const principal = factory.getPrincipal({ cwd: dir, paths, compilerOptions, compilers });
const principal = factory.getPrincipal({ cwd: dir, paths, compilerOptions, compilers, pkgName });

const worker = new WorkspaceWorker({
name,
Expand Down Expand Up @@ -245,23 +245,36 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

debugLog(`Installed ${principals.length} principals for ${workspaces.length} workspaces`);

for (const principal of principals) {
const exportedSymbols: Exports = new Map();
const importedSymbols: Imports = new Map();
const analyzedFiles: Set<string> = new Set();
const exportedSymbols: Exports = new Map();
const importedSymbols: Imports = new Map();

const analyzeSourceFile = (filePath: string) => {
for (const principal of principals) {
const analyzeSourceFile = (filePath: string, _principal: ProjectPrincipal = principal) => {
const workspace = chief.findWorkspaceByFilePath(filePath);
if (workspace) {
const { imports, exports, scripts } = principal.analyzeSourceFile(filePath, { skipTypeOnly: isProduction });
const { imports, exports, scripts } = _principal.analyzeSourceFile(filePath, { skipTypeOnly: isProduction });
const { internal, external, unresolved } = imports;
const { exported, duplicate } = exports;

if (exported.size > 0) exportedSymbols.set(filePath, exported);

for (const [specifierFilePath, importItems] of internal.entries()) {
// Mark "external" imports from other local workspaces as used dependency
const packageName = getPackageNameFromModuleSpecifier(importItems.specifier);
if (packageName && chief.localWorkspaces.has(packageName)) external.add(packageName);
if (packageName && chief.localWorkspaces.has(packageName)) {
// Mark "external" imports from other local workspaces as used dependency
external.add(packageName);
if (_principal === principal) {
const workspace = chief.findWorkspaceByFilePath(specifierFilePath);
if (workspace) {
const principal = factory.getPrincipalByPackageName(workspace.pkgName);
if (principal && !principal.isGitIgnored(specifierFilePath)) {
analyzeSourceFile(specifierFilePath, principal);
analyzedFiles.add(specifierFilePath);
}
}
}
}

if (!importedSymbols.has(specifierFilePath)) {
importedSymbols.set(specifierFilePath, importItems);
Expand Down Expand Up @@ -297,7 +310,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
});

_getDependenciesFromScripts(scripts, { cwd: dirname(filePath) }).forEach(specifier => {
handleReferencedDependency({ specifier, containingFilePath: filePath, principal, workspace });
handleReferencedDependency({ specifier, containingFilePath: filePath, principal: _principal, workspace });
});
}
};
Expand All @@ -308,7 +321,6 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

streamer.cast('Connecting the dots...');

const analyzedFiles: Set<string> = new Set();
let size = principal.entryPaths.size;
let round = 0;

Expand All @@ -323,44 +335,44 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
analyzedFiles.add(filePath);
});
} while (size !== principal.entryPaths.size);
}

const unusedFiles = principal.getUnreferencedFiles();

collector.addFilesIssues(unusedFiles);

collector.addFileCounts({ processed: analyzedFiles.size, unused: unusedFiles.length });
const isSymbolImported = (symbol: string, importingModule?: ImportedModule): boolean => {
if (!importingModule) return false;
if (importingModule.symbols.has(symbol)) return true;
const { isReExport, isReExportedBy } = importingModule;
const hasSymbol = (file: string) => isSymbolImported(symbol, importedSymbols.get(file));
return isReExport ? Array.from(isReExportedBy).some(hasSymbol) : false;
};

const isSymbolImported = (symbol: string, importingModule?: ImportedModule): boolean => {
if (!importingModule) return false;
if (importingModule.symbols.has(symbol)) return true;
const { isReExport, isReExportedBy } = importingModule;
const hasSymbol = (file: string) => isSymbolImported(symbol, importedSymbols.get(file));
return isReExport ? Array.from(isReExportedBy).some(hasSymbol) : false;
};
const isExportedInEntryFile = (principal: ProjectPrincipal, importedModule?: ImportedModule): boolean => {
if (!importedModule) return false;
const { isReExport, isReExportedBy } = importedModule;
const { entryPaths } = principal;
const hasFile = (file: string) =>
entryPaths.has(file) || isExportedInEntryFile(principal, importedSymbols.get(file));
return isReExport ? Array.from(isReExportedBy).some(hasFile) : false;
};

const isExportedInEntryFile = (importedModule?: ImportedModule): boolean => {
if (!importedModule) return false;
const { isReExport, isReExportedBy } = importedModule;
const { entryPaths } = principal;
const hasFile = (file: string) => entryPaths.has(file) || isExportedInEntryFile(importedSymbols.get(file));
return isReExport ? Array.from(isReExportedBy).some(hasFile) : false;
};
const isExportedItemReferenced = (principal: ProjectPrincipal, exportedItem: ExportItem, filePath: string) => {
const hasReferences = principal.getHasReferences(filePath, exportedItem);
return (
hasReferences.external ||
(hasReferences.internal &&
(typeof chief.config.ignoreExportsUsedInFile === 'object'
? exportedItem.type !== 'unknown' && !!chief.config.ignoreExportsUsedInFile[exportedItem.type]
: chief.config.ignoreExportsUsedInFile))
);
};

const isExportedItemReferenced = (exportedItem: ExportItem, filePath: string) => {
const hasReferences = principal.getHasReferences(filePath, exportedItem);
return (
hasReferences.external ||
(hasReferences.internal &&
(typeof chief.config.ignoreExportsUsedInFile === 'object'
? exportedItem.type !== 'unknown' && !!chief.config.ignoreExportsUsedInFile[exportedItem.type]
: chief.config.ignoreExportsUsedInFile))
);
};
if (isReportValues || isReportTypes) {
streamer.cast('Analyzing source files...');

if (isReportValues || isReportTypes) {
streamer.cast('Analyzing source files...');
for (const [filePath, exportItems] of exportedSymbols.entries()) {
const workspace = chief.findWorkspaceByFilePath(filePath);
const principal = workspace && factory.getPrincipalByPackageName(workspace.pkgName);

for (const [filePath, exportItems] of exportedSymbols.entries()) {
if (principal) {
// Bail out when in entry file (unless --include-entry-exports)
if (!isIncludeEntryExports && principal.entryPaths.has(filePath)) continue;

Expand All @@ -375,7 +387,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

if (importingModule && isSymbolImported(symbol, importingModule)) {
// Skip members of classes/enums that are eventually exported by entry files
if (importingModule.isReExport && isExportedInEntryFile(importingModule)) continue;
if (importingModule.isReExport && isExportedInEntryFile(principal, importingModule)) continue;

if (report.enumMembers && exportedItem.type === 'enum' && exportedItem.members) {
if (isProduction) continue;
Expand All @@ -395,9 +407,10 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
}

const isStar = Boolean(importingModule?.isStar);
const isReExportedByEntryFile = !isIncludeEntryExports && isStar && isExportedInEntryFile(importingModule);
const isReExportedByEntryFile =
!isIncludeEntryExports && isStar && isExportedInEntryFile(principal, importingModule);

if (!isReExportedByEntryFile && !isExportedItemReferenced(exportedItem, filePath)) {
if (!isReExportedByEntryFile && !isExportedItemReferenced(principal, exportedItem, filePath)) {
if (['enum', 'type', 'interface'].includes(exportedItem.type)) {
if (isProduction) continue;
const type = isStar ? 'nsTypes' : 'types';
Expand All @@ -412,6 +425,15 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
}
}

const unusedFiles = factory
.getPrincipals()
.flatMap(principal => principal.getUnreferencedFiles())
.filter(filePath => !analyzedFiles.has(filePath));

collector.addFilesIssues(unusedFiles);

collector.addFileCounts({ processed: analyzedFiles.size, unused: unusedFiles.length });

if (isReportDependencies) {
const { dependencyIssues, devDependencyIssues, optionalPeerDependencyIssues } = deputy.settleDependencyIssues();
const { configurationHints } = deputy.getConfigurationHints();
Expand Down
25 changes: 25 additions & 0 deletions tests/workspaces-cross-reference.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { main } from '../src/index.js';
import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

const cwd = resolve('fixtures/workspaces-cross-reference');

test('Resolve imports in separate workspaces without entry file', async () => {
const { issues, counters } = await main({
...baseArguments,
cwd,
});

assert(issues.exports['packages/lib-a/mod-a.ts']['unused']);
assert(issues.exports['packages/lib-b/mod-b.ts']['unused']);

assert.deepEqual(counters, {
...baseCounters,
exports: 2,
processed: 4,
total: 4,
});
});

0 comments on commit 8ab8992

Please sign in to comment.