Skip to content

Commit

Permalink
Consider module.exports = require() to be re-exports
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Sep 23, 2024
1 parent e40f618 commit e2bdb95
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 5 deletions.
6 changes: 6 additions & 0 deletions packages/knip/fixtures/re-exports-cjs/1-entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const { something } = require('./2-re-export-star');
something;

module.exports.somethingToIgnore = require('./2-re-export-star').somethingToIgnore;

module.exports.somethingNotToIgnore = require('./2-re-export-star').somethingNotToIgnore;
1 change: 1 addition & 0 deletions packages/knip/fixtures/re-exports-cjs/2-re-export-star.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./3-my-module');
3 changes: 3 additions & 0 deletions packages/knip/fixtures/re-exports-cjs/3-my-module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports.something = {};
module.exports.somethingToIgnore = {};
module.exports.somethingNotToIgnore = {};
7 changes: 7 additions & 0 deletions packages/knip/fixtures/re-exports-cjs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@fixtures/re-exports-cjs",
"knip": {
"entry": ["1-entry.js"],
"project": ["*.js"]
}
}
8 changes: 8 additions & 0 deletions packages/knip/src/typescript/ast-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,11 @@ export const getExportKeywordNode = (node: ts.Node) =>
export const getDefaultKeywordNode = (node: ts.Node) =>
// @ts-expect-error Property 'modifiers' does not exist on type 'Node'.
(node.modifiers as ts.Modifier[])?.find(mod => mod.kind === ts.SyntaxKind.DefaultKeyword);

export const hasRequireCall = (node: ts.Node): boolean => {
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'require') return true;
return node.getChildren().some(child => hasRequireCall(child));
};

export const isModuleExportsAccess = (node: ts.PropertyAccessExpression) =>
ts.isIdentifier(node.expression) && node.expression.escapedText === 'module' && node.name.escapedText === 'exports';
1 change: 1 addition & 0 deletions packages/knip/src/typescript/get-imports-and-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ const getImportsAndExports = (
// Patterns:
// export { id } from 'specifier';
// export * from 'specifier';
// module.exports = require('specifier');
addValue(imports.reExported, identifier, sourceFile.fileName);
}
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ts from 'typescript';
import { findAncestor, findDescendants, isRequireCall, isTopLevel } from '../../ast-helpers.js';
import { IMPORT_STAR } from '../../../constants.js';
import { findAncestor, findDescendants, isModuleExportsAccess, isRequireCall, isTopLevel } from '../../ast-helpers.js';
import { isNotJS } from '../helpers.js';
import { importVisitor as visit } from '../index.js';

Expand Down Expand Up @@ -56,6 +57,16 @@ export default visit(
// Pattern: require('specifier')
return { identifier: 'default', specifier, pos: node.arguments[0].pos, resolve };
}

if (
ts.isBinaryExpression(node.parent) &&
ts.isPropertyAccessExpression(node.parent.left) &&
isModuleExportsAccess(node.parent.left)
) {
// Pattern: module.exports = require('specifier')
return { identifier: IMPORT_STAR, specifier, isReExport: true, pos: node.arguments[0].pos };
}

// Pattern: require('side-effects')
return { identifier: 'default', specifier, pos: node.arguments[0].pos, resolve };
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import ts from 'typescript';
import type { Fix } from '../../../types/exports.js';
import { SymbolType } from '../../../types/issues.js';
import { stripQuotes } from '../../ast-helpers.js';
import { hasRequireCall, isModuleExportsAccess, stripQuotes } from '../../ast-helpers.js';
import { isJS } from '../helpers.js';
import { exportVisitor as visit } from '../index.js';

const isModuleExportsAccess = (node: ts.PropertyAccessExpression) =>
ts.isIdentifier(node.expression) && node.expression.escapedText === 'module' && node.name.escapedText === 'exports';

export default visit(isJS, (node, { isFixExports }) => {
if (ts.isExpressionStatement(node)) {
if (ts.isBinaryExpression(node.expression)) {
Expand Down Expand Up @@ -37,6 +34,12 @@ export default visit(isJS, (node, { isFixExports }) => {
return { node, identifier: node.getText(), type: SymbolType.UNKNOWN, pos: node.getStart(), fix };
});
}

if (ts.isCallExpression(node.expression.right) && hasRequireCall(node.expression.right)) {
// Pattern: module.exports = require('specifier');
return;
}

// Pattern: module.exports = any
return { node, identifier: 'default', type: SymbolType.UNKNOWN, pos: expr.pos + 1, fix: undefined };
}
Expand Down
21 changes: 21 additions & 0 deletions packages/knip/test/re-exports-cjs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { test } from 'bun:test';
import assert from 'node:assert/strict';
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/re-exports-cjs');

test('Ignore re-exports from entry files (CommonJS', async () => {
const { counters } = await main({
...baseArguments,
cwd,
});

assert.deepEqual(counters, {
...baseCounters,
processed: 3,
total: 3,
});
});

0 comments on commit e2bdb95

Please sign in to comment.