Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support commonjs-like directory imports #6

Merged
merged 9 commits into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ npx codemod@latest correct-ts-specifiers
* `.js` → `.d.cts`, `.d.mts`, or `.d.ts`
* Package.json subimports
* tsconfig paths (requires a loader)
* Commonjs-like directory specifiers

Before:

Expand All @@ -48,9 +49,10 @@ import { URL } from 'node:url';
import { bar } from '@dep/bar';
import { foo } from 'foo';

import { Bird } from './Bird'; // a directory
import { Cat } from './Cat.ts';
import { Dog } from '…/Dog/index.mjs'; // tsconfig paths
import { baseUrl } from '#config.js'; // package.json imports
import { Dog } from '…/Dog/index.mjs'; // tsconfig paths
import { baseUrl } from '#config.js'; // package.json imports

export { Zed } from './zed';

Expand All @@ -60,6 +62,7 @@ export const makeLink = (path: URL) => (new URL(path, baseUrl)).href;

const nil = await import('./nil.js');

const bird = new Bird('Tweety');
const cat = new Cat('Milo');
const dog = new Dog('Otis');
```
Expand All @@ -72,9 +75,10 @@ import { URL } from 'node:url';
import { bar } from '@dep/bar';
import { foo } from 'foo';

import { Bird } from './Bird/index.ts';
import { Cat } from './Cat.ts';
import { Dog } from '…/Dog/index.mts'; // tsconfig paths
import { baseUrl } from '#config.js'; // package.json imports
import { Dog } from '…/Dog/index.mts'; // tsconfig paths
import { baseUrl } from '#config.js'; // package.json imports

export type { Zed } from './zed.d.ts';

Expand All @@ -84,16 +88,7 @@ export const makeLink = (path: URL) => (new URL(path, baseUrl)).href;

const nil = await import('./nil.ts');

const bird = new Bird('Tweety');
const cat = new Cat('Milo');
const dog = new Dog('Otis');
```

## Unsupported cases

* Directory / commonjs-like specifiers¹

```ts
import foo from '..'; // where '..' → '../index.ts' (or similar)
```

¹ Support may be added in a future release
18 changes: 18 additions & 0 deletions src/exts.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
/**
* A map of JavaScript file extensions to the corresponding TypeScript file extension.
*/
export const jsToTSExts = {
'.cjs': '.cts',
'.mjs': '.mts',
'.js': '.ts',
'.jsx': '.tsx',
} as const;
/**
* File extensions that potentially need to be corrected
*/
export const suspectExts = {
'': '.js',
...jsToTSExts,
Expand All @@ -14,9 +20,21 @@ export const jsExts = Object.keys(jsToTSExts) as Array<JSExt>;
export const tsExts = Object.values(jsToTSExts);
export type TSExt = typeof tsExts[number];

/**
* File extensions for TypeScript type declaration files.
*/
export const dExts = [
'.d.cts',
'.d.ts',
'.d.mts',
] as const;
export type DExt = typeof dExts[number];

/**
* A master list of file extensions to check.
*/
export const extSets = new Set([
jsExts,
tsExts,
dExts,
]);
62 changes: 33 additions & 29 deletions src/fexists.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import assert from 'node:assert/strict';
import {
type Mock,
after,
afterEach,
before,
describe,
Expand All @@ -13,7 +12,8 @@ import { fileURLToPath } from 'node:url';

type FSAccess = typeof import('node:fs/promises').access;
type FExists = typeof import('./fexists.ts').fexists;
type MockModuleContext = ReturnType<typeof mock.module>;
type ResolveSpecifier = typeof import('./resolve-specifier.ts').resolveSpecifier;
// type MockModuleContext = ReturnType<typeof mock.module>;

const RESOLVED_SPECIFIER_ERR = 'Resolved specifier did not match expected';

Expand All @@ -22,21 +22,26 @@ describe('fexists', () => {
const constants = { F_OK: null };

let mock__access: Mock<FSAccess>['mock'];
let mock__fs: MockModuleContext;
let mock__resolveSpecifier: Mock<ResolveSpecifier>['mock'];

before(() => {
const access = mock.fn<FSAccess>();
({ mock: mock__access } = access);
mock__fs = mock.module('node:fs/promises', {
mock.module('node:fs/promises', {
namedExports: {
access,
constants,
},
});
});

after(() => {
mock__fs.restore();
const resolveSpecifier = mock.fn<ResolveSpecifier>();
({ mock: mock__resolveSpecifier } = resolveSpecifier);
mock.module('./resolve-specifier.ts', {
namedExports: {
resolveSpecifier,
},
});
mock__resolveSpecifier.mockImplementation(function MOCK__resolveSpecifier(pp, specifier) { return specifier });
});

describe('when the file exists', () => {
Expand All @@ -52,35 +57,34 @@ describe('fexists', () => {
mock__access.resetCalls();
});

after(() => {
mock__fs.restore();
});

it('should return `true` for a bare specifier', async () => {
const specifier = 'foo';
const parentUrl = fileURLToPath(import.meta.resolve('./fixtures/e2e/test.js'));

assert.equal(await fexists(parentUrl, 'foo'), true);
assert.equal(await fexists(parentUrl, specifier), true);
assert.equal(
mock__access.calls[0].arguments[0],
fileURLToPath(import.meta.resolve('./fixtures/e2e/node_modules/foo/foo.js')),
specifier,
RESOLVED_SPECIFIER_ERR,
);
});

it('should return `true` for a relative specifier', async () => {
assert.equal(await fexists(parentPath, 'exists.js'), true);
const specifier = 'exists.js';
assert.equal(await fexists(parentPath, specifier), true);
assert.equal(
mock__access.calls[0].arguments[0],
'/tmp/exists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});

it('should return `true` for specifier with a query parameter', async () => {
assert.equal(await fexists(parentPath, 'exists.js?v=1'), true);
const specifier = 'exists.js?v=1';
assert.equal(await fexists(parentPath, specifier), true);
assert.equal(
mock__access.calls[0].arguments[0],
'/tmp/exists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});
Expand Down Expand Up @@ -119,42 +123,42 @@ describe('fexists', () => {
mock__access.resetCalls();
});

after(() => {
mock__fs.restore();
});

it('should return `false` for a relative specifier', async () => {
assert.equal(await fexists(parentPath, 'noexists.js'), false);
const specifier = 'noexists.js';
assert.equal(await fexists(parentPath, specifier), false);
assert.equal(
mock__access.calls[0].arguments[0],
'/tmp/noexists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});

it('should return `false` for a relative specifier', async () => {
assert.equal(await fexists(parentPath, 'noexists.js?v=1'), false);
const specifier = 'noexists.js?v=1';
assert.equal(await fexists(parentPath, specifier), false);
assert.equal(
mock__access.calls[0].arguments[0],
'/tmp/noexists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});

it('should return `false` for an absolute specifier', async () => {
assert.equal(await fexists(parentPath, '/tmp/foo/noexists.js'), false);
const specifier = '/tmp/foo/noexists.js';
assert.equal(await fexists(parentPath, specifier), false);
assert.equal(
mock__access.calls[0].arguments[0],
'/tmp/foo/noexists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});

it('should return `false` for a URL specifier', async () => {
assert.equal(await fexists(parentPath, 'file://localhost/foo/noexists.js'), false);
const specifier = 'file://localhost/foo/noexists.js';
assert.equal(await fexists(parentPath, specifier), false);
assert.equal(
mock__access.calls[0].arguments[0],
'file://localhost/foo/noexists.js',
specifier,
RESOLVED_SPECIFIER_ERR,
);
});
Expand Down
12 changes: 2 additions & 10 deletions src/fexists.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
import { dirname } from 'node:path';
import { access, constants } from 'node:fs/promises';
import { fileURLToPath, pathToFileURL } from 'node:url';

import type { FSAbsolutePath, Specifier } from './index.d.ts';
import { resolveSpecifier } from './resolve-specifier.ts';


export function fexists(
parentPath: FSAbsolutePath,
specifier: Specifier,
) {
const parentUrl = `${pathToFileURL(dirname(parentPath)).href}/`;

const resolvedSpecifier: FSAbsolutePath = URL.canParse(specifier)
? specifier
// import.meta.resolve gives access to node's resolution algorithm, which is necessary to handle
// a myriad of non-obvious routes, like pJson subimports and the result of any hooks that may be
// helping, such as ones facilitating tsconfig's "paths"
: fileURLToPath(import.meta.resolve(specifier, parentUrl));
const resolvedSpecifier = resolveSpecifier(parentPath, specifier);

return access(
resolvedSpecifier,
Expand Down
Empty file added src/fixtures/dir/cjs/index.cjs
Empty file.
Empty file added src/fixtures/dir/cts/index.cts
Empty file.
Empty file added src/fixtures/dir/js/index.js
Empty file.
Empty file added src/fixtures/dir/jsx/index.jsx
Empty file.
Empty file added src/fixtures/dir/mjs/index.mjs
Empty file.
Empty file added src/fixtures/dir/mts/index.mts
Empty file.
Empty file added src/fixtures/dir/ts/index.ts
Empty file.
Empty file added src/fixtures/dir/tsx/index.tsx
Empty file.
3 changes: 3 additions & 0 deletions src/fixtures/e2e/Bird/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export class Bird {
constructor(public name: string) {}
}
3 changes: 3 additions & 0 deletions src/fixtures/e2e/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { URL } from 'node:url';
import { bar } from '@dep/bar';
import { foo } from 'foo';

import { Bird } from './Bird';
import { Cat } from './Cat.ts';
import { Dog } from '…/Dog/index.mjs';
import { baseUrl } from '#config.js';
Expand All @@ -15,9 +16,11 @@ export const makeLink = (path: URL) => (new URL(path, baseUrl)).href;

const nil = await import('./nil.js');

const bird = new Bird('Tweety');
const cat = new Cat('Milo');
const dog = new Dog('Otis');

console.log('bird:', bird);
console.log('cat:', cat);
console.log('dog:', dog);
console.log('foo:', foo);
Expand Down
19 changes: 19 additions & 0 deletions src/is-dir.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { lstat } from 'fs/promises';

import type { FSAbsolutePath, Specifier } from './index.d.ts';
import { resolveSpecifier } from './resolve-specifier.ts';


export async function isDir(
parentPath: FSAbsolutePath,
specifier: Specifier,
) {
const resolvedSpecifier = resolveSpecifier(parentPath, specifier);

try {
const stat = await lstat(resolvedSpecifier);
return stat.isDirectory();
} catch (err) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import { fileURLToPath } from 'node:url';

import { tsExts } from './exts.ts';
import { isIgnorableSpecifier } from './isIgnorableSpecifier.ts';
import { isIgnorableSpecifier } from './is-ignorable-specifier.ts';


describe('Is ignorable specifier', () => {
Expand Down
5 changes: 5 additions & 0 deletions src/isIgnorableSpecifier.ts → src/is-ignorable-specifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import { tsExts } from './exts.ts';
import type { FSAbsolutePath } from './index.d.ts';


/**
* Whether the specifier can be completely ignored.
* @param parentPath The module containing the provided specifier
* @param specifier The specifier to check.
*/
export function isIgnorableSpecifier(
parentPath: FSAbsolutePath,
specifier: string,
Expand Down
2 changes: 1 addition & 1 deletion src/map-imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('Map Imports', () => {
specifier,
);

assert.equal(output.replacement, undefined);
assert.equal(output.replacement, specifier);
assert.notEqual(output.isType, true);
});

Expand Down
17 changes: 11 additions & 6 deletions src/map-imports.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import type { FSAbsolutePath, Specifier } from './index.d.ts';
import { fexists } from './fexists.ts';
import { logger } from './logger.js';
import { isIgnorableSpecifier } from './isIgnorableSpecifier.ts';
import { isIgnorableSpecifier } from './is-ignorable-specifier.ts';
import { replaceJSExtWithTSExt } from './replace-js-ext-with-ts-ext.ts';
import { isDir } from './is-dir.ts';


/**
* Determine what, if anything, to replace the existing specifier.
* @param parentPath The module containing the provided specifier.
* @param specifier The specifier to potentially correct.
*/
export const mapImports = async (
parentPath: FSAbsolutePath,
specifier: Specifier,
Expand All @@ -17,7 +23,10 @@ export const mapImports = async (
let { isType, replacement } = await replaceJSExtWithTSExt(parentPath, specifier);

if (replacement) {
if (await fexists(parentPath, specifier)) {
if (
await fexists(parentPath, specifier)
&& !(await isDir(parentPath, specifier))
) {
logger(
parentPath,
'warn', [
Expand All @@ -31,10 +40,6 @@ export const mapImports = async (
return { isType, replacement };
}

({ replacement } = await replaceJSExtWithTSExt(parentPath, specifier, '.d.ts'));

if (replacement) return { isType, replacement };

if (!await fexists(parentPath, specifier)) logger(
parentPath,
'error',
Expand Down
Loading
Loading