Skip to content

Commit

Permalink
Refactor module resolution for non-std files + group related tests (f…
Browse files Browse the repository at this point in the history
…ixes #376)
  • Loading branch information
webpro committed Dec 2, 2023
1 parent e7c26d9 commit 1154a09
Show file tree
Hide file tree
Showing 51 changed files with 76 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "@fixtures/module-resolution-baseurl-implicit-relative"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import Image from 'assets/image.svg';
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'dir/main';
import { helloWorld } from 'hello/world';

helloWorld;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "@fixtures/module-resolution-non-std-implicit"
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import ImageSVG from '../assets/image.svg';
import ImagePNG from '../assets/image.png';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import 'dir/main.ts';

import SomeSVG from './common/image.svg';

import Icon from './icon.svg';

import './global.css';

// By exception, .css files will be resolved because `.css` is not added to virtual path extensions, since `.css.ts`
// files on disk are common (in contrast to e.g. `.svg.ts`; binaries like `.png.ts` can be safely ignored).
import 'styles/base.css';
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@fixtures/unresolved",
"name": "@fixtures/module-resolution-non-std",
"dependencies": {
"@svg-icons/fa-brands": "*",
"@svg-icons/heroicons-outline": "*"
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Empty file.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Github from '@svg-icons/fa-brands/github.svg';
import Briefcase from '@svg-icons/heroicons-outline/briefcase.svg';

import SomeSVG from 'common/image.svg';
import SomePNG from 'common/image.png';

import './globals.css';
import 'styles/base.css';
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@fixtures/module-resolution-tsconfig-paths",
"dependencies": {
"internal": "*"
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@fixture/subpath-import-from-plugin",
"name": "@fixtures/subpath-import-from-plugin",
"imports": {
"#dep": {
"node": "dep-node-native",
Expand Down
3 changes: 0 additions & 3 deletions packages/knip/fixtures/tsconfig-paths-implicit/package.json

This file was deleted.

6 changes: 0 additions & 6 deletions packages/knip/fixtures/tsconfig-paths/package.json

This file was deleted.

11 changes: 9 additions & 2 deletions packages/knip/src/ConfigurationChief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import mapWorkspaces from '@npmcli/map-workspaces';
import { createPkgGraph } from '@pnpm/workspace.pkgs-graph';
import micromatch from 'micromatch';
import { ConfigurationValidator } from './ConfigurationValidator.js';
import { ROOT_WORKSPACE_NAME, DEFAULT_EXTENSIONS, KNIP_CONFIG_LOCATIONS } from './constants.js';
import {
ROOT_WORKSPACE_NAME,
DEFAULT_EXTENSIONS,
KNIP_CONFIG_LOCATIONS,
DUMMY_VIRTUAL_FILE_EXTENSIONS,
} from './constants.js';
import { defaultRules } from './issues/initializers.js';
import * as plugins from './plugins/index.js';
import { arrayify } from './util/array.js';
Expand Down Expand Up @@ -180,7 +185,9 @@ export class ConfigurationChief {
}

public getCompilers(): [SyncCompilers, AsyncCompilers] {
return [this.config.syncCompilers, this.config.asyncCompilers];
const syncCompilers = this.config.syncCompilers;
DUMMY_VIRTUAL_FILE_EXTENSIONS.forEach(ext => !syncCompilers.has(ext) && syncCompilers.set(ext, () => ''));
return [syncCompilers, this.config.asyncCompilers];
}

public getRules() {
Expand Down
4 changes: 2 additions & 2 deletions packages/knip/src/ProjectPrincipal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class ProjectPrincipal {
...compilerOptions,
...baseCompilerOptions,
types: compact([...(compilerOptions.types ?? []), ...baseCompilerOptions.types]),
allowNonTsExtensions: [...compilers].flat().length > 0,
allowNonTsExtensions: true,
};

const [syncCompilers, asyncCompilers] = compilers;
Expand Down Expand Up @@ -220,7 +220,7 @@ export class ProjectPrincipal {
const isIgnored = this.isGitIgnored(join(dirname(filePath), sanitizedSpecifier));
if (!isIgnored) {
const ext = extname(sanitizedSpecifier);
const hasIgnoredExtension = IGNORED_FILE_EXTENSIONS.includes(ext);
const hasIgnoredExtension = IGNORED_FILE_EXTENSIONS.has(ext);
if (!ext || (ext !== '.json' && !hasIgnoredExtension)) {
unresolvedImports.add(unresolvedImport);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/src/binaries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const getDependenciesFromScripts: GetDependenciesFromScripts = (npmScripts, opti
if (isBinary(identifier)) return identifier;
if (isInternal(identifier)) {
const ext = extname(identifier);
if (ext && IGNORED_FILE_EXTENSIONS.includes(ext)) return;
if (ext && IGNORED_FILE_EXTENSIONS.has(ext)) return;
return identifier;
}
return getPackageNameFromModuleSpecifier(identifier);
Expand Down
14 changes: 6 additions & 8 deletions packages/knip/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,26 @@ export const IGNORED_GLOBAL_BINARIES = [

export const IGNORED_DEPENDENCIES = ['knip', 'typescript'];

export const IGNORED_FILE_EXTENSIONS = [
// 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([
'.avif',
'.css',
'.eot',
'.gif',
'.html',
'.ico',
'.jpeg',
'.jpg',
'.less',
'.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
export const IGNORE_DEFINITELY_TYPED = ['node'];
Expand Down
2 changes: 1 addition & 1 deletion packages/knip/test/cli-reporter-json2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { resolve } from '../src/util/path.js';
import { execFactory } from './helpers/exec.js';
import { updatePos } from './helpers/index.js';

const cwd = resolve('fixtures/unresolved');
const cwd = resolve('fixtures/module-resolution-non-std');

const exec = execFactory(cwd);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
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/module-resolution-baseurl-implicit-relative');

test('Resolve implicit relative module specifiers from baseUrl', async () => {
const { counters } = await main({
...baseArguments,
cwd,
});

assert.deepEqual(counters, {
...baseCounters,
processed: 3,
total: 3,
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

test('Resolve modules properly using tsconfig paths with baseUrl', async () => {
const cwd = resolve('fixtures/tsconfig-paths-implicit');
const cwd = resolve('fixtures/module-resolution-non-std-implicit');

test('Resolve non-standard relative specifiers (no tsconfig.json)', async () => {
const { counters } = await main({
...baseArguments,
cwd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

const cwd = resolve('fixtures/unresolved');
const cwd = resolve('fixtures/module-resolution-non-std');

test('Report unresolved imports', async () => {
test('Resolve non-standard extensions and report unresolved imports', async () => {
const { issues, counters } = await main({
...baseArguments,
cwd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

test('Resolve modules properly using tsconfig paths and globs', async () => {
const cwd = resolve('fixtures/tsconfig-paths');
const cwd = resolve('fixtures/module-resolution-tsconfig-paths');

test('Resolve modules properly using tsconfig paths and globs', async () => {
const { issues, counters } = await main({
...baseArguments,
cwd,
Expand Down

0 comments on commit 1154a09

Please sign in to comment.