Skip to content

Commit

Permalink
Refactor module resolution (closes #504, closes #511, closes #550)
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Mar 18, 2024
1 parent 3d65079 commit 318d28e
Show file tree
Hide file tree
Showing 33 changed files with 165 additions and 78 deletions.
3 changes: 2 additions & 1 deletion packages/knip/fixtures/custom-paths-workspaces/knip.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
},
"ws": {
"paths": {
"#util/*": ["./util/*"],
"#util-foo/*": ["./util/*"],
"~images/*": ["./assets/img/*"],
"lib/*": ["./lib/*"]
}
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion packages/knip/fixtures/custom-paths-workspaces/ws/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import main from 'lib/main';
import lang from '#util/lang';
import lang from '#util-foo/lang';
import svg from '~images/logo.svg';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const Button: () => null;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const Button = () => null;
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import h from 'dir/subdir';
import { g } from 'utils/fn';
import { fn } from 'utils/fn';
import { obj } from 'utils/obj.cjs';
import { str } from 'utils/str.mjs';
import { Button } from 'components/Button';

h;
g;
fn;
obj;
str;
Button;
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const g: () => void;
export const fn: () => void;
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const g = () => {};
export const fn = () => {};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const obj = () => {};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const obj: () => void;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const str: () => void;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const str = () => {};
9 changes: 9 additions & 0 deletions packages/knip/fixtures/dts-compiled/knip.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { KnipConfig } from 'knip';

const config: KnipConfig = {
compilers: {
graphql: () => '',
},
};

export default config;
6 changes: 6 additions & 0 deletions packages/knip/fixtures/dts-compiled/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "knip-issue-non-js-dts",
"private": true,
"version": "0.0.0",
"type": "module"
}
14 changes: 14 additions & 0 deletions packages/knip/fixtures/dts-compiled/src/App.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import ExampleQuery from './ExampleQuery.graphql';

function App() {
return (
<>
<h1>Vite + React</h1>
<pre>
<code>{JSON.stringify(ExampleQuery, null, 2)}</code>
</pre>
</>
);
}

export default App;
6 changes: 6 additions & 0 deletions packages/knip/fixtures/dts-compiled/src/ExampleQuery.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
query ExampleQuery {
example {
name
description
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as Types from './types';

export type ExampleQueryQueryVariables = Types.Exact<{ [key: string]: never }>;

export type ExampleQueryQuery = {
__typename?: 'Query';
example?: { __typename?: 'Example'; name?: string | null; description?: string | null } | null;
};
5 changes: 5 additions & 0 deletions packages/knip/fixtures/dts-compiled/src/UnusedQuery.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
query UnusedQuery {
example {
name
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as Types from './types';

export type UnusedQueryQueryVariables = Types.Exact<{ [key: string]: never }>;

export type UnusedQueryQuery = {
__typename?: 'Query';
example?: { __typename?: 'Example'; name?: string | null } | null;
};
2 changes: 2 additions & 0 deletions packages/knip/fixtures/dts-compiled/src/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import App from './App.tsx';
App;
1 change: 1 addition & 0 deletions packages/knip/fixtures/dts-compiled/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] };
1 change: 1 addition & 0 deletions packages/knip/fixtures/dts-compiled/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
9 changes: 3 additions & 6 deletions packages/knip/src/ProjectPrincipal.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import ts from 'typescript';
import { dummyCompilers, getCompilerExtensions } from './compilers/index.js';
import { DEFAULT_EXTENSIONS } from './constants.js';
import { IGNORED_FILE_EXTENSIONS } from './constants.js';
import { getCompilerExtensions } from './compilers/index.js';
import { DEFAULT_EXTENSIONS, FOREIGN_FILE_EXTENSIONS } from './constants.js';
import { createHosts } from './typescript/createHosts.js';
import { _getImportsAndExports } from './typescript/getImportsAndExports.js';
import { createCustomModuleResolver } from './typescript/resolveModuleNames.js';
Expand Down Expand Up @@ -104,8 +103,6 @@ export class ProjectPrincipal {
}

init() {
this.addCompilers([dummyCompilers, new Map()]);

const { fileManager, compilerHost, resolveModuleNames, languageServiceHost } = createHosts({
cwd: this.cwd,
compilerOptions: this.compilerOptions,
Expand Down Expand Up @@ -253,7 +250,7 @@ export class ProjectPrincipal {
const isIgnored = this.isGitIgnored(join(dirname(filePath), sanitizedSpecifier));
if (!isIgnored) {
const ext = extname(sanitizedSpecifier);
const hasIgnoredExtension = IGNORED_FILE_EXTENSIONS.has(ext);
const hasIgnoredExtension = FOREIGN_FILE_EXTENSIONS.has(ext);
if (!ext || (ext !== '.json' && !hasIgnoredExtension)) {
unresolvedImports.add(unresolvedImport);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/knip/src/binaries/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IGNORED_FILE_EXTENSIONS } from '../constants.js';
import { FOREIGN_FILE_EXTENSIONS } from '../constants.js';
import { compact } from '../util/array.js';
import { getPackageNameFromModuleSpecifier } from '../util/modules.js';
import { extname, isInternal } from '../util/path.js';
Expand All @@ -20,7 +20,7 @@ const getDependenciesFromScripts: GetDependenciesFromScripts = (npmScripts, opti
}
if (isInternal(identifier)) {
const ext = extname(identifier);
if (ext && IGNORED_FILE_EXTENSIONS.has(ext)) return;
if (ext && FOREIGN_FILE_EXTENSIONS.has(ext)) return;
return identifier;
}
return getPackageNameFromModuleSpecifier(identifier);
Expand Down
5 changes: 0 additions & 5 deletions packages/knip/src/compilers/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { DUMMY_VIRTUAL_FILE_EXTENSIONS } from '../constants.js';
import Astro from './astro.js';
import MDX from './mdx.js';
import Svelte from './svelte.js';
Expand Down Expand Up @@ -40,10 +39,6 @@ const compilers = new Map([
['.vue', Vue],
]);

const dummyCompiler: SyncCompilerFn = () => '';

export const dummyCompilers = new Map(Array.from(DUMMY_VIRTUAL_FILE_EXTENSIONS).map(ext => [ext, dummyCompiler]));

export const getIncludedCompilers = (
syncCompilers: SyncCompilers,
asyncCompilers: AsyncCompilers,
Expand Down
14 changes: 8 additions & 6 deletions packages/knip/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,27 @@ export const IGNORED_DEPENDENCIES = new Set(['knip', 'typescript']);

export const IGNORED_RUNTIME_DEPENDENCIES = new Set(['bun']);

// Extensions sometimes imported directly (and compiled away by bundlers, etc.)
export const DUMMY_VIRTUAL_FILE_EXTENSIONS = new Set(['.html', '.jpeg', '.jpg', '.png', '.svg', '.webp']);

export const IGNORED_FILE_EXTENSIONS = new Set([
export const FOREIGN_FILE_EXTENSIONS = new Set([
'.avif',
'.css',
'.eot',
'.gif',
'.html',
'.ico',
'.jpeg',
'.jpg',
'.less',
'.mp3',
'.png',
'.sass',
'.scss',
'.sh',
'.svg',
'.ttf',
'.webp',
'.woff',
'.woff2',
'.yaml',
'.yml',
...DUMMY_VIRTUAL_FILE_EXTENSIONS,
]);

// The `@types/node` dependency does not require the `node` dependency
Expand Down
2 changes: 2 additions & 0 deletions packages/knip/src/typescript/SourceFileManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import ts from 'typescript';
import { FOREIGN_FILE_EXTENSIONS } from '../constants.js';
import { debugLog } from '../util/debug.js';
import { extname, isInNodeModules, isInternal } from '../util/path.js';
import type { SyncCompilers, AsyncCompilers } from '../compilers/types.js';
Expand Down Expand Up @@ -29,6 +30,7 @@ export class SourceFileManager {
}

getSourceFile(filePath: string) {
if (FOREIGN_FILE_EXTENSIONS.has(extname(filePath))) return undefined;
if (this.isSkipLibs && isInNodeModules(filePath)) return undefined;
if (this.sourceFileCache.has(filePath)) return this.sourceFileCache.get(filePath);
const contents = ts.sys.readFile(filePath);
Expand Down
7 changes: 4 additions & 3 deletions packages/knip/src/typescript/createHosts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EOL } from 'node:os';
import path from 'node:path';
import ts from 'typescript';
import { getCompilerExtensions } from '../compilers/index.js';
import { FOREIGN_FILE_EXTENSIONS } from '../constants.js';
import { createCustomModuleResolver } from './resolveModuleNames.js';
import { SourceFileManager } from './SourceFileManager.js';
import { createCustomSys } from './sys.js';
Expand All @@ -20,9 +21,9 @@ type CreateHostsOptions = {

export const createHosts = ({ cwd, compilerOptions, entryPaths, compilers, isSkipLibs }: CreateHostsOptions) => {
const fileManager = new SourceFileManager({ compilers, isSkipLibs });
const virtualFileExtensions = getCompilerExtensions(compilers);
const sys = createCustomSys(cwd, virtualFileExtensions);
const resolveModuleNames = createCustomModuleResolver(sys, compilerOptions, virtualFileExtensions);
const compilerExtensions = [...getCompilerExtensions(compilers), ...FOREIGN_FILE_EXTENSIONS];
const sys = createCustomSys(cwd, compilerExtensions);
const resolveModuleNames = createCustomModuleResolver(sys, compilerOptions, compilerExtensions);

const languageServiceHost: ts.LanguageServiceHost = {
getCompilationSettings: () => compilerOptions,
Expand Down
85 changes: 38 additions & 47 deletions packages/knip/src/typescript/resolveModuleNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { existsSync } from 'node:fs';
import { isBuiltin } from 'node:module';
import ts from 'typescript';
import { sanitizeSpecifier } from '../util/modules.js';
import { basename, dirname, extname, format, isAbsolute, isInNodeModules, isInternal, join } from '../util/path.js';
import { dirname, extname, isAbsolute, isInNodeModules, isInternal, join } from '../util/path.js';
import { isDeclarationFileExtension } from './ast-helpers.js';
import { ensureRealFilePath, isVirtualFilePath } from './utils.js';

Expand All @@ -20,30 +20,6 @@ const fileExists = (name: string, containingFile: string) => {
}
};

const DTS_EXTENSIONS_MAP = {
[ts.Extension.Dts]: ts.Extension.Js,
[ts.Extension.Dmts]: ts.Extension.Mjs,
[ts.Extension.Dcts]: ts.Extension.Cjs,
} as const;

const jsMatchingDeclarationFileExists = (resolveDtsFileName: string, dtsExtension: string) => {
const extension = DTS_EXTENSIONS_MAP[dtsExtension as keyof typeof DTS_EXTENSIONS_MAP];
const resolvedFileName = format({
ext: extension,
dir: dirname(resolveDtsFileName),
name: basename(resolveDtsFileName, dtsExtension),
});

if (existsSync(resolvedFileName)) {
return {
resolvedFileName,
extension,
isExternalLibraryImport: false,
resolvedUsingTsExtension: false,
};
}
};

export function createCustomModuleResolver(
customSys: typeof ts.sys,
compilerOptions: ts.CompilerOptions,
Expand All @@ -64,9 +40,8 @@ export function createCustomModuleResolver(
function resolveModuleName(name: string, containingFile: string): ts.ResolvedModuleFull | undefined {
const sanitizedSpecifier = sanitizeSpecifier(name);

// No need to try and resolve builtins, bail out
if (isBuiltin(sanitizedSpecifier)) return undefined;
if (isInNodeModules(name)) return undefined;
// No need to try and resolve builtins or externals, bail out
if (isBuiltin(sanitizedSpecifier) || isInNodeModules(name)) return undefined;

const tsResolvedModule = ts.resolveModuleName(
sanitizedSpecifier,
Expand All @@ -75,33 +50,47 @@ export function createCustomModuleResolver(
ts.sys
).resolvedModule;

// When TS does not resolve it, and it's not a registered virtual file ext, try `fs.existsSync`
if (!tsResolvedModule) {
const extension = extname(sanitizedSpecifier);
if (extension && !virtualFileExtensions.includes(extension)) {
const module = fileExists(sanitizedSpecifier, containingFile);
if (module) return module;
}
}

// This turns resolved local `.d.ts` filenames into `.js` (if specifier has the extension and file exists),
// because there can be both module.d.ts and module.js and we want the latter.
if (
tsResolvedModule &&
isDeclarationFileExtension(tsResolvedModule.extension) &&
isInternal(tsResolvedModule.resolvedFileName)
) {
{
const module = jsMatchingDeclarationFileExists(tsResolvedModule.resolvedFileName, tsResolvedModule.extension);
if (module) return module;
if (tsResolvedModule.extension === '.d.mts') {
const resolvedFileName = tsResolvedModule.resolvedFileName.replace(/\.d\.mts$/, '.mjs');
return { resolvedFileName, extension: '.mjs', isExternalLibraryImport: false, resolvedUsingTsExtension: false };
}
{
const module = fileExists(sanitizedSpecifier, containingFile);
if (module) return module;

if (tsResolvedModule.extension === '.d.cts') {
const resolvedFileName = tsResolvedModule.resolvedFileName.replace(/\.d\.cts$/, '.cjs');
return { resolvedFileName, extension: '.cjs', isExternalLibraryImport: false, resolvedUsingTsExtension: false };
}
}

if (virtualFileExtensions.length === 0) return tsResolvedModule;
const base = tsResolvedModule.resolvedFileName.replace(/\.d\.ts$/, '');
const baseExt = extname(base);

if (virtualFileExtensions.includes(baseExt)) {
const resolvedFileName = ensureRealFilePath(base, virtualFileExtensions);
return {
extension: ts.Extension.Js,
resolvedFileName,
isExternalLibraryImport: false,
resolvedUsingTsExtension: false,
};
}

for (const e of ['.js', '.jsx']) {
if (existsSync(base + e)) {
return {
resolvedFileName: base + e,
extension: e,
isExternalLibraryImport: false,
resolvedUsingTsExtension: false,
};
}
}

return tsResolvedModule;
}

if (tsResolvedModule && !isVirtualFilePath(tsResolvedModule.resolvedFileName, virtualFileExtensions)) {
return tsResolvedModule;
Expand All @@ -115,6 +104,8 @@ export function createCustomModuleResolver(
).resolvedModule;

if (!customResolvedModule || !isVirtualFilePath(customResolvedModule.resolvedFileName, virtualFileExtensions)) {
const module = fileExists(sanitizedSpecifier, containingFile);
if (module) return module;
return customResolvedModule;
}

Expand Down
File renamed without changes.
4 changes: 2 additions & 2 deletions packages/knip/test/dts-baseurl-implicit-relative.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test('Include js files referred by the declaration files', async () => {

assert.deepEqual(counters, {
...baseCounters,
processed: 5,
total: 5,
processed: 11,
total: 11,
});
});
Loading

0 comments on commit 318d28e

Please sign in to comment.