Skip to content

Commit

Permalink
Improve re-export handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@webpro
webpro committed Jun 9, 2024
1 parent b0b8b3d commit 9ccefb3
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 45 deletions.
15 changes: 11 additions & 4 deletions packages/knip/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,19 +408,26 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
const importsForExport = file.imported;

for (const [identifier, exportedItem] of exportItems.entries()) {
if (exportedItem.isReExport) continue;

// Skip tagged exports
if (shouldIgnore(exportedItem.jsDocTags)) continue;

if (importsForExport) {
const { isReferenced, hasReExportingEntryFile, traceNode } = isIdentifierReferenced(
const { isReferenced, reExportingEntryFile, traceNode } = isIdentifierReferenced(
filePath,
identifier,
isIncludeEntryExports
);

if (!isIncludeEntryExports && hasReExportingEntryFile) {
createAndPrintTrace(filePath, { identifier, isEntry, hasRef: isReferenced });
continue;
if (reExportingEntryFile) {
if (!isIncludeEntryExports) {
createAndPrintTrace(filePath, { identifier, isEntry, hasRef: isReferenced });
continue;
}
// Skip exports if re-exported from entry file and tagged
const reExportedItem = graph.get(reExportingEntryFile)?.exports.exported.get(identifier);
if (reExportedItem && shouldIgnore(reExportedItem.jsDocTags)) continue;
}

if (traceNode) printTrace(traceNode, filePath, identifier);
Expand Down
1 change: 1 addition & 0 deletions packages/knip/src/types/dependency-graph.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface Export {
refs: number;
fixes: Fixes;
symbol?: ts.Symbol;
isReExport: boolean;
}

export type ExportMember = {
Expand Down
39 changes: 23 additions & 16 deletions packages/knip/src/typescript/getImportsAndExports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,34 +103,35 @@ const getImportsAndExports = (

if (!file) internalImports.set(filePath, imports);

const nsOrAlias = symbol ? String(symbol.escapedName) : options.alias;

if (isReExport) {
if (isStar && namespace) {
// Pattern: export * as NS from 'specifier';
addValue(imports.reExportedNs, namespace, sourceFile.fileName);
} else if (namespace) {
} else if (nsOrAlias) {
// Pattern: export { id as alias } from 'specifier';
addNsValue(imports.reExportedAs, identifier, namespace, sourceFile.fileName);
addNsValue(imports.reExportedAs, identifier, nsOrAlias, sourceFile.fileName);
} else {
// Patterns:
// export { id } from 'specifier';
// export * from 'specifier';
addValue(imports.reExported, identifier, sourceFile.fileName);
}
}

const nsOrAlias = symbol ? String(symbol.escapedName) : options.alias;
if (nsOrAlias && nsOrAlias !== identifier) {
if (isStar) {
addValue(imports.importedNs, nsOrAlias, sourceFile.fileName);
} else {
addNsValue(imports.importedAs, identifier, nsOrAlias, sourceFile.fileName);
} else {
if (nsOrAlias && nsOrAlias !== identifier) {
if (isStar) {
addValue(imports.importedNs, nsOrAlias, sourceFile.fileName);
} else {
addNsValue(imports.importedAs, identifier, nsOrAlias, sourceFile.fileName);
}
} else if (identifier !== ANONYMOUS && identifier !== IMPORT_STAR) {
addValue(imports.imported, identifier, sourceFile.fileName);
}
} else if (identifier !== ANONYMOUS && identifier !== IMPORT_STAR) {
addValue(imports.imported, identifier, sourceFile.fileName);
}

if (symbol && DEFAULT_EXTENSIONS.includes(extname(sourceFile.fileName))) {
importedInternalSymbols.set(symbol, filePath);
if (symbol && DEFAULT_EXTENSIONS.includes(extname(sourceFile.fileName))) {
importedInternalSymbols.set(symbol, filePath);
}
}
};

Expand Down Expand Up @@ -228,12 +229,17 @@ const getImportsAndExports = (

const exportMembers = members.map(member => createMember(node, member, member.pos));

const isReExport = Boolean(
node.parent?.parent && ts.isExportDeclaration(node.parent.parent) && node.parent.parent.moduleSpecifier
);

const item = exports.get(identifier);
if (item) {
// Code path for fn overloads, simple merge
const members = [...(item.members ?? []), ...exportMembers];
const tags = new Set([...(item.jsDocTags ?? []), ...jsDocTags]);
const fixes = fix ? [...(item.fixes ?? []), fix] : item.fixes;
exports.set(identifier, { ...item, members, jsDocTags: tags, fixes });
exports.set(identifier, { ...item, members, jsDocTags: tags, fixes, isReExport });
} else {
const { line, character } = node.getSourceFile().getLineAndCharacterOfPosition(pos);
exports.set(identifier, {
Expand All @@ -248,6 +254,7 @@ const getImportsAndExports = (
col: character + 1,
fixes: fix ? [fix] : [],
refs: 0,
isReExport,
});
}

Expand Down
38 changes: 19 additions & 19 deletions packages/knip/src/util/is-identifier-referenced.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { type TraceNode, addNodes, createNode, isTrace } from './trace.js';

type Result = {
isReferenced: boolean;
hasReExportingEntryFile: boolean;
reExportingEntryFile: undefined | string;
traceNode: TraceNode;
};

Expand All @@ -17,11 +17,11 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
seen = new Set<string>()
): Result => {
let isReferenced = false;
let hasReExportingEntryFile = entryPaths.has(filePath);
let reExportingEntryFile = entryPaths.has(filePath) ? filePath : undefined;

if (hasReExportingEntryFile) traceNode.isEntry = true;
if (reExportingEntryFile) traceNode.isEntry = true;

if (!isIncludeEntryExports && hasReExportingEntryFile) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isIncludeEntryExports && reExportingEntryFile) return { isReferenced, reExportingEntryFile, traceNode };

seen.add(filePath);

Expand All @@ -30,14 +30,14 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa

const file = graph.get(filePath)?.imported;

if (!file) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!file) return { isReferenced, reExportingEntryFile, traceNode };

if (
((identifier !== id && file.refs.has(id)) || identifier === id) &&
(file.imported.has(identifier) || file.importedAs.has(identifier))
) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
addNodes(traceNode, id, graph, file.imported.get(identifier));
}

Expand All @@ -47,7 +47,7 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
const aliasedRef = [alias, ...rest].join('.');
if (file.refs.has(aliasedRef)) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
addNodes(traceNode, aliasedRef, graph, aliases.get(alias));
}
}
Expand All @@ -57,7 +57,7 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
for (const [namespace, byFilePaths] of file.importedNs) {
if (file.refs.has(`${namespace}.${id}`)) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
addNodes(traceNode, `${namespace}.${id}`, graph, byFilePaths);
}

Expand All @@ -70,10 +70,10 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
const child = createNode(byFilePath);
traceNode.children.add(child);
const result = isIdentifierReferenced(byFilePath, `${alias}.${id}`, isIncludeEntryExports, child, seen);
if (result.hasReExportingEntryFile) hasReExportingEntryFile = true;
if (result.reExportingEntryFile) reExportingEntryFile = result.reExportingEntryFile;
if (result.isReferenced) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
}
}
}
Expand All @@ -88,10 +88,10 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
const child = createNode(byFilePath);
traceNode.children.add(child);
const result = isIdentifierReferenced(byFilePath, `${namespace}.${id}`, isIncludeEntryExports, child, seen);
if (result.hasReExportingEntryFile) hasReExportingEntryFile = true;
if (result.reExportingEntryFile) reExportingEntryFile = result.reExportingEntryFile;
if (result.isReferenced) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
}
}
}
Expand All @@ -106,10 +106,10 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
const child = createNode(byFilePath);
traceNode.children.add(child);
const result = isIdentifierReferenced(byFilePath, id, isIncludeEntryExports, child, seen);
if (result.hasReExportingEntryFile) hasReExportingEntryFile = true;
if (result.reExportingEntryFile) reExportingEntryFile = result.reExportingEntryFile;
if (result.isReferenced) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
}
}
}
Expand All @@ -125,10 +125,10 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
traceNode.children.add(child);
const ref = [alias, ...rest].join('.');
const result = isIdentifierReferenced(byFilePath, ref, isIncludeEntryExports, child, seen);
if (result.hasReExportingEntryFile) hasReExportingEntryFile = true;
if (result.reExportingEntryFile) reExportingEntryFile = result.reExportingEntryFile;
if (result.isReferenced) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
}
}
}
Expand All @@ -141,16 +141,16 @@ export const getIsIdentifierReferencedHandler = (graph: DependencyGraph, entryPa
const child = createNode(byFilePath);
traceNode.children.add(child);
const result = isIdentifierReferenced(byFilePath, `${namespace}.${id}`, isIncludeEntryExports, child, seen);
if (result.hasReExportingEntryFile) hasReExportingEntryFile = true;
if (result.reExportingEntryFile) reExportingEntryFile = result.reExportingEntryFile;
if (result.isReferenced) {
isReferenced = true;
if (!isTrace) return { isReferenced, hasReExportingEntryFile, traceNode };
if (!isTrace) return { isReferenced, reExportingEntryFile, traceNode };
}
}
}
}

return { isReferenced, hasReExportingEntryFile, traceNode };
return { isReferenced, reExportingEntryFile, traceNode };
};

return isIdentifierReferenced;
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/test/fix-workspaces.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const cwd = resolve('fixtures/fix-workspaces');

const readContents = async (fileName: string) => await readFile(join(cwd, fileName), 'utf8');

test('Remove exports and dependencies', async () => {
test('Remove exports and dependencies (workspaces)', async () => {
const tests = [
[
'exports.ts',
Expand Down
12 changes: 10 additions & 2 deletions packages/knip/test/fix.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ module.exports = { identifier, };
[
'reexports.js',
await readContents('reexports.js'),
`export { One } from './reexported';
`export { RangeSlider } from './reexported';
export { Rating } from './reexported';
export { One } from './reexported';
export { Col, Col as KCol } from './reexported';
export { Row as KRow, Row } from './reexported';
`.replace(/\n/g, EOL),
],
[
Expand Down Expand Up @@ -89,14 +93,18 @@ export const One = 1;
tags: [[], ['knipignore']],
});

assert(issues.exports['access.js']['ACCESS']);
assert(issues.exports['access.js']['UNUSED']);
assert(issues.exports['access.js']['ACCESS']);
assert(issues.exports['exports.js']['identifier2']);
assert(issues.exports['mod.ts']['a']);
assert(issues.exports['mod.ts']['b']);
assert(issues.exports['mod.ts']['c']);
assert(issues.exports['mod.ts']['d']);
assert(issues.exports['mod.ts']['default']);
assert(issues.exports['mod.ts']['x']);
assert(issues.exports['mod.ts']['y']);
assert(issues.exports['reexported.js']['Three']);
assert(issues.exports['reexported.js']['Two']);

// check ignore
assert(issues.exports['ignored.ts'] === undefined);
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/test/re-exports-with-decorator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import baseCounters from './helpers/baseCounters.js';

const cwd = resolve('fixtures/re-exports-with-decorator');

test('Ignore re-exports from entry files', async () => {
test('Ignore re-exports from entry file (w/ decorator)', async () => {
const { counters } = await main({
...baseArguments,
cwd,
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/test/re-exports.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ test('Ignore re-exports from entry files (include entry + ignore @public)', asyn
isIncludeEntryExports: true,
});

assert(issues.exports['1-entry.ts']['somethingNotToIgnore']);
assert(issues.exports['4-my-module.ts']['somethingNotToIgnore']);

assert.deepEqual(counters, {
...baseCounters,
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/test/workspaces-dts.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test('Find unused un-built exports across workspaces', async () => {
cwd,
});

assert(issues.exports['packages/shared/src/index.js']['unusedFunction']);
assert(issues.exports['packages/shared/src/unused-function.js']['unusedFunction']);

assert.deepEqual(counters, {
...baseCounters,
Expand Down

0 comments on commit 9ccefb3

Please sign in to comment.