From a0157fdddea97b984d3cb9ded154185fb2fd13ec Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:02:50 +0200 Subject: [PATCH 1/8] chore: extract `resolveSpecifier` into own module --- src/fexists.ts | 12 ++---------- src/resolveSpecifier.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 src/resolveSpecifier.ts diff --git a/src/fexists.ts b/src/fexists.ts index 7c5084c..cf17fab 100644 --- a/src/fexists.ts +++ b/src/fexists.ts @@ -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 './resolveSpecifier.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, diff --git a/src/resolveSpecifier.ts b/src/resolveSpecifier.ts new file mode 100644 index 0000000..435d76a --- /dev/null +++ b/src/resolveSpecifier.ts @@ -0,0 +1,21 @@ +import { dirname } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +import type { FSAbsolutePath, Specifier } from './index.d.ts'; + + +export function resolveSpecifier( + parentPath: FSAbsolutePath, + specifier: Specifier, +): FSAbsolutePath { + 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)); + + return resolvedSpecifier; +} From 2337523b616b5e684c2f9fbff7a34539754abd80 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:03:11 +0200 Subject: [PATCH 2/8] feat: create `isDir` --- src/isDir.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/isDir.ts diff --git a/src/isDir.ts b/src/isDir.ts new file mode 100644 index 0000000..491d988 --- /dev/null +++ b/src/isDir.ts @@ -0,0 +1,19 @@ +import { lstat } from 'fs/promises'; + +import type { FSAbsolutePath, Specifier } from './index.d.ts'; +import { resolveSpecifier } from './resolveSpecifier.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; + } +} From a7d0519935fb37eb08d0588e6f8540050298bef0 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:30:00 +0200 Subject: [PATCH 3/8] feat: support transforming commonjs-like directory imports --- README.md | 23 ++++----- src/exts.ts | 6 +++ src/fixtures/dir/cjs/index.cjs | 0 src/fixtures/dir/cts/index.cts | 0 src/fixtures/dir/js/index.js | 0 src/fixtures/dir/jsx/index.jsx | 0 src/fixtures/dir/mjs/index.mjs | 0 src/fixtures/dir/mts/index.mts | 0 src/fixtures/dir/ts/index.ts | 0 src/fixtures/dir/tsx/index.tsx | 0 src/fixtures/e2e/Bird/index.ts | 3 ++ src/fixtures/e2e/test.ts | 3 ++ src/map-imports.test.ts | 2 +- src/map-imports.ts | 10 ++-- src/replace-js-ext-with-ts-ext.test.ts | 64 +++++++++++++++++++++++++ src/replace-js-ext-with-ts-ext.ts | 65 +++++++++++++++++++------- src/workflow.test.ts.snapshot | 2 +- 17 files changed, 139 insertions(+), 39 deletions(-) create mode 100644 src/fixtures/dir/cjs/index.cjs create mode 100644 src/fixtures/dir/cts/index.cts create mode 100644 src/fixtures/dir/js/index.js create mode 100644 src/fixtures/dir/jsx/index.jsx create mode 100644 src/fixtures/dir/mjs/index.mjs create mode 100644 src/fixtures/dir/mts/index.mts create mode 100644 src/fixtures/dir/ts/index.ts create mode 100644 src/fixtures/dir/tsx/index.tsx create mode 100644 src/fixtures/e2e/Bird/index.ts diff --git a/README.md b/README.md index 2e9cafb..36607d0 100644 --- a/README.md +++ b/README.md @@ -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) +* Directory / commonjs-like specifiers Before: @@ -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'; @@ -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'); ``` @@ -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'; @@ -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 diff --git a/src/exts.ts b/src/exts.ts index 2587629..8b7cfc6 100644 --- a/src/exts.ts +++ b/src/exts.ts @@ -20,3 +20,9 @@ export const dExts = [ '.d.mts', ] as const; export type DExt = typeof dExts[number]; + +export const extSets = new Set([ + jsExts, + tsExts, + dExts, +]); diff --git a/src/fixtures/dir/cjs/index.cjs b/src/fixtures/dir/cjs/index.cjs new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/cts/index.cts b/src/fixtures/dir/cts/index.cts new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/js/index.js b/src/fixtures/dir/js/index.js new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/jsx/index.jsx b/src/fixtures/dir/jsx/index.jsx new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/mjs/index.mjs b/src/fixtures/dir/mjs/index.mjs new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/mts/index.mts b/src/fixtures/dir/mts/index.mts new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/ts/index.ts b/src/fixtures/dir/ts/index.ts new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/dir/tsx/index.tsx b/src/fixtures/dir/tsx/index.tsx new file mode 100644 index 0000000..e69de29 diff --git a/src/fixtures/e2e/Bird/index.ts b/src/fixtures/e2e/Bird/index.ts new file mode 100644 index 0000000..1af437c --- /dev/null +++ b/src/fixtures/e2e/Bird/index.ts @@ -0,0 +1,3 @@ +export class Bird { + constructor(public name: string) {} +} diff --git a/src/fixtures/e2e/test.ts b/src/fixtures/e2e/test.ts index c83041d..d4f7139 100644 --- a/src/fixtures/e2e/test.ts +++ b/src/fixtures/e2e/test.ts @@ -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'; @@ -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); diff --git a/src/map-imports.test.ts b/src/map-imports.test.ts index 54ba9b4..a9c02f0 100644 --- a/src/map-imports.test.ts +++ b/src/map-imports.test.ts @@ -95,7 +95,7 @@ describe('Map Imports', () => { specifier, ); - assert.equal(output.replacement, undefined); + assert.equal(output.replacement, specifier); assert.notEqual(output.isType, true); }); diff --git a/src/map-imports.ts b/src/map-imports.ts index 44e04ef..8bd12b3 100644 --- a/src/map-imports.ts +++ b/src/map-imports.ts @@ -3,6 +3,7 @@ import { fexists } from './fexists.ts'; import { logger } from './logger.js'; import { isIgnorableSpecifier } from './isIgnorableSpecifier.ts'; import { replaceJSExtWithTSExt } from './replace-js-ext-with-ts-ext.ts'; +import { isDir } from './isDir.ts'; export const mapImports = async ( @@ -17,7 +18,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', [ @@ -31,10 +35,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', diff --git a/src/replace-js-ext-with-ts-ext.test.ts b/src/replace-js-ext-with-ts-ext.test.ts index b5c1e67..ff82319 100644 --- a/src/replace-js-ext-with-ts-ext.test.ts +++ b/src/replace-js-ext-with-ts-ext.test.ts @@ -14,6 +14,7 @@ import { dExts, jsExts, suspectExts, + tsExts, } from './exts.ts'; @@ -110,4 +111,67 @@ describe('Correcting ts file extensions', () => { } }); }); + + describe('specifier is inherently a directory', () => { + it('should attempt to find an index file', async () => { + for (const base of ['.', './']) { + for (const jsExt of jsExts) { + const output = await replaceJSExtWithTSExt( + fileURLToPath(import.meta.resolve(`./fixtures/dir/${jsExt.slice(1)}/test.ts`)), + base, + ); + + assert.equal(output.replacement, `${base}${base.endsWith('/') ? '' : '/'}index${jsExt}`); + assert.equal(output.isType, false); + } + + for (const tsExt of tsExts) { + const output = await replaceJSExtWithTSExt( + fileURLToPath(import.meta.resolve(`./fixtures/dir/${tsExt.slice(1)}/test.ts`)), + base, + ); + + assert.equal(output.replacement, `${base}${base.endsWith('/') ? '' : '/'}index${tsExt}`); + assert.equal(output.isType, false); + } + } + }); + }); + + describe('specifier is NOT inherently a directory', () => { + it('should attempt to find an index file', async () => { + for (const dExt of dExts) { + const base = `./fixtures/d/unambiguous/${dExt.slice(3)}`; + const output = await replaceJSExtWithTSExt( + originatingFilePath, + base, + ); + + assert.equal(output.replacement, `${base}/index${dExt}`); + assert.equal(output.isType, true); + } + + for (const jsExt of jsExts) { + const base = `./fixtures/dir/${jsExt.slice(1)}`; + const output = await replaceJSExtWithTSExt( + originatingFilePath, + base, + ); + + assert.equal(output.replacement, `${base}/index${jsExt}`); + assert.equal(output.isType, false); + } + + for (const tsExt of tsExts) { + const base = `./fixtures/dir/${tsExt.slice(1)}`; + const output = await replaceJSExtWithTSExt( + originatingFilePath, + base, + ); + + assert.equal(output.replacement, `${base}/index${tsExt}`); + assert.equal(output.isType, false); + } + }); + }); }); diff --git a/src/replace-js-ext-with-ts-ext.ts b/src/replace-js-ext-with-ts-ext.ts index 12c1e55..a955212 100644 --- a/src/replace-js-ext-with-ts-ext.ts +++ b/src/replace-js-ext-with-ts-ext.ts @@ -5,11 +5,12 @@ import { type DExt, type JSExt, type TSExt, + extSets, suspectExts, - dExts, } from './exts.ts'; import { fexists } from './fexists.ts'; import { logger } from './logger.js'; +import { isDir } from './isDir.ts'; export const replaceJSExtWithTSExt = async ( @@ -20,6 +21,19 @@ export const replaceJSExtWithTSExt = async ( isType?: boolean, replacement: FSAbsolutePath | null, }> => { + if ( + specifier === '.' + || specifier === '..' + ) { + specifier += '/index'; + } else if ( + specifier.endsWith('/') + || await isDir(parentPath, specifier) + ) { + if (!specifier.endsWith('/')) specifier += '/'; + specifier += 'index'; + } + if (!extname(specifier)) { specifier += '.js'; @@ -32,21 +46,9 @@ export const replaceJSExtWithTSExt = async ( if (await fexists(parentPath, replacement)) return { replacement, isType: false }; - const dFound = new Set(); - for (const dExt of dExts) { - const potential = composeReplacement(specifier, oExt, dExt); - if (await fexists(parentPath, potential)) dFound.add(replacement = potential); - } - - if (dFound.size) { - if (dFound.size === 1) return { replacement, isType: true }; - - logger( - parentPath, - 'error', [ - `"${specifier}" appears to resolve to a declaration file, but multiple declaration files`, - `were found. Cannot disambiguate between "${Array.from(dFound).join('", "')}" (skipping).`, - ].join(' ')); + for (const extSet of extSets) { + const result = await checkSet(parentPath, specifier, oExt, extSet); + if (result) return result; } return { replacement: null }; @@ -55,7 +57,34 @@ export const replaceJSExtWithTSExt = async ( const composeReplacement = ( specifier:Specifier, oExt: JSExt, - rExt: TSExt | DExt, -) => oExt + rExt: DExt | JSExt | TSExt, +): Specifier => oExt ? specifier.replace(oExt, rExt) : `${specifier}${rExt}`; + +async function checkSet( + parentPath: FSAbsolutePath, + specifier: Specifier, + oExt: JSExt, + exts: Array, +) { + let replacement: Specifier; + + const found = new Set(); + for (const ext of exts) { + const potential = composeReplacement(specifier, oExt, ext); + if (await fexists(parentPath, potential)) found.add(replacement = potential); + } + + if (found.size) { + if (found.size === 1) return { replacement, isType: exts[0].startsWith('.d') }; + + logger( + parentPath, + 'error', [ + `"${specifier}" appears to resolve to multiple files. Cannot disambiguate between`, + `"${Array.from(found).join('", "')}"`, + '(skipping).', + ].join(' ')); + } +} diff --git a/src/workflow.test.ts.snapshot b/src/workflow.test.ts.snapshot index 4fa9909..d30da90 100644 --- a/src/workflow.test.ts.snapshot +++ b/src/workflow.test.ts.snapshot @@ -1,3 +1,3 @@ exports[`workflow > should update bad specifiers and ignore good ones 1`] = ` -"import { URL } from 'node:url';\\n\\nimport { bar } from '@dep/bar';\\nimport { foo } from 'foo';\\n\\nimport { Cat } from './Cat.ts';\\nimport { Dog } from '…/Dog/index.mts';\\nimport { baseUrl } from '#config.js';\\n\\nexport type { Zed } from './zed.d.ts';\\n\\n// should be unchanged\\n\\nexport const makeLink = (path: URL) => (new URL(path, baseUrl)).href;\\n\\nconst nil = await import('./nil.ts');\\n\\nconst cat = new Cat('Milo');\\nconst dog = new Dog('Otis');\\n\\nconsole.log('cat:', cat);\\nconsole.log('dog:', dog);\\nconsole.log('foo:', foo);\\nconsole.log('bar:', bar);\\nconsole.log('nil:', nil);\\n" +"import { URL } from 'node:url';\\n\\nimport { bar } from '@dep/bar';\\nimport { foo } from 'foo';\\n\\nimport { Bird } from './Bird/index.ts';\\nimport { Cat } from './Cat.ts';\\nimport { Dog } from '…/Dog/index.mts';\\nimport { baseUrl } from '#config.js';\\n\\nexport type { Zed } from './zed.d.ts';\\n\\n// should be unchanged\\n\\nexport const makeLink = (path: URL) => (new URL(path, baseUrl)).href;\\n\\nconst nil = await import('./nil.ts');\\n\\nconst bird = new Bird('Tweety');\\nconst cat = new Cat('Milo');\\nconst dog = new Dog('Otis');\\n\\nconsole.log('bird:', bird);\\nconsole.log('cat:', cat);\\nconsole.log('dog:', dog);\\nconsole.log('foo:', foo);\\nconsole.log('bar:', bar);\\nconsole.log('nil:', nil);\\n" `; From 21a7dcf4859ea90bab2efe250a0d0ed72d334fb8 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:39:54 +0200 Subject: [PATCH 4/8] doc: wordsmith supported cases list --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 36607d0..ead14d4 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ npx codemod@latest correct-ts-specifiers * `.js` → `.d.cts`, `.d.mts`, or `.d.ts` * Package.json subimports * tsconfig paths (requires a loader) -* Directory / commonjs-like specifiers +* Commonjs-like directory specifiers Before: From 8b8586a42086e5088b96caed8513f1977e780a76 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:50:44 +0200 Subject: [PATCH 5/8] WIP --- src/resolveSpecifier.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/resolveSpecifier.ts b/src/resolveSpecifier.ts index 435d76a..ef17b93 100644 --- a/src/resolveSpecifier.ts +++ b/src/resolveSpecifier.ts @@ -10,12 +10,21 @@ export function resolveSpecifier( ): FSAbsolutePath { 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)); - - return resolvedSpecifier; + if (URL.canParse(specifier)) return 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" + let resolvedSpecifierUrl: URL['href']; + + try { + resolvedSpecifierUrl = import.meta.resolve(specifier, parentUrl); + } catch (err) { + console.error({ specifier, parentPath }); + throw err; + } + + if (!resolvedSpecifierUrl.startsWith('file://')) return specifier; + + return fileURLToPath(resolvedSpecifierUrl) as FSAbsolutePath; } From 50c4a5a333cf5acb0ed800a27ab730301d83a2a4 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 27 Oct 2024 13:30:02 +0100 Subject: [PATCH 6/8] chore: fix file names & add code docs --- package.json | 2 +- src/exts.ts | 12 ++++++++ src/fexists.ts | 2 +- src/{isDir.ts => is-dir.ts} | 2 +- ...test.ts => is-ignorable-specifier.test.ts} | 2 +- ...Specifier.ts => is-ignorable-specifier.ts} | 5 ++++ src/map-imports.ts | 9 ++++-- src/replace-js-ext-with-ts-ext.ts | 28 +++++++++++++++++-- ...solveSpecifier.ts => resolve-specifier.ts} | 7 ++++- 9 files changed, 60 insertions(+), 9 deletions(-) rename src/{isDir.ts => is-dir.ts} (86%) rename src/{isIgnorableSpecifier.test.ts => is-ignorable-specifier.test.ts} (97%) rename src/{isIgnorableSpecifier.ts => is-ignorable-specifier.ts} (89%) rename src/{resolveSpecifier.ts => resolve-specifier.ts} (76%) diff --git a/package.json b/package.json index 8dd77e0..571e1a3 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ }, "scripts": { "start": "node --no-warnings --experimental-import-meta-resolve --experimental-strip-types ./src/workflow.ts", - "test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --experimental-strip-types --test './**/*.test.ts'" + "test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --experimental-strip-types './**/*.test.ts'" }, "repository": { "type": "git", diff --git a/src/exts.ts b/src/exts.ts index 8b7cfc6..c700528 100644 --- a/src/exts.ts +++ b/src/exts.ts @@ -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, @@ -14,6 +20,9 @@ export const jsExts = Object.keys(jsToTSExts) as Array; 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', @@ -21,6 +30,9 @@ export const dExts = [ ] as const; export type DExt = typeof dExts[number]; +/** + * A master list of file extensions to check. + */ export const extSets = new Set([ jsExts, tsExts, diff --git a/src/fexists.ts b/src/fexists.ts index cf17fab..1296652 100644 --- a/src/fexists.ts +++ b/src/fexists.ts @@ -1,7 +1,7 @@ import { access, constants } from 'node:fs/promises'; import type { FSAbsolutePath, Specifier } from './index.d.ts'; -import { resolveSpecifier } from './resolveSpecifier.ts'; +import { resolveSpecifier } from './resolve-specifier.ts'; export function fexists( diff --git a/src/isDir.ts b/src/is-dir.ts similarity index 86% rename from src/isDir.ts rename to src/is-dir.ts index 491d988..68aef01 100644 --- a/src/isDir.ts +++ b/src/is-dir.ts @@ -1,7 +1,7 @@ import { lstat } from 'fs/promises'; import type { FSAbsolutePath, Specifier } from './index.d.ts'; -import { resolveSpecifier } from './resolveSpecifier.ts'; +import { resolveSpecifier } from './resolve-specifier.ts'; export async function isDir( diff --git a/src/isIgnorableSpecifier.test.ts b/src/is-ignorable-specifier.test.ts similarity index 97% rename from src/isIgnorableSpecifier.test.ts rename to src/is-ignorable-specifier.test.ts index f630ec3..b6e097c 100644 --- a/src/isIgnorableSpecifier.test.ts +++ b/src/is-ignorable-specifier.test.ts @@ -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', () => { diff --git a/src/isIgnorableSpecifier.ts b/src/is-ignorable-specifier.ts similarity index 89% rename from src/isIgnorableSpecifier.ts rename to src/is-ignorable-specifier.ts index 9763e9e..716879a 100644 --- a/src/isIgnorableSpecifier.ts +++ b/src/is-ignorable-specifier.ts @@ -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, diff --git a/src/map-imports.ts b/src/map-imports.ts index 8bd12b3..de2575a 100644 --- a/src/map-imports.ts +++ b/src/map-imports.ts @@ -1,11 +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 './isDir.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, diff --git a/src/replace-js-ext-with-ts-ext.ts b/src/replace-js-ext-with-ts-ext.ts index a955212..39f1a54 100644 --- a/src/replace-js-ext-with-ts-ext.ts +++ b/src/replace-js-ext-with-ts-ext.ts @@ -10,9 +10,16 @@ import { } from './exts.ts'; import { fexists } from './fexists.ts'; import { logger } from './logger.js'; -import { isDir } from './isDir.ts'; +import { isDir } from './is-dir.ts'; +/** + * Attempts to find a TypeScript-related file at the specifier's location. When there are multiple + * potential matches, a message is logged and the specifier is skipped. + * @param parentPath The module containing the provided specifier. + * @param specifier The specifier to potentially correct. + * @param rExt A file extension to try to use when making a correction. + */ export const replaceJSExtWithTSExt = async ( parentPath: FSAbsolutePath, specifier: Specifier, @@ -54,6 +61,13 @@ export const replaceJSExtWithTSExt = async ( return { replacement: null }; }; +/** + * Composes a new specifier with the original file extension replaced with the new (or appends the + * new when there was no original). + * @param specifier The specifier to update. + * @param oExt The original/current extension. + * @param rExt The replacement extension. + */ const composeReplacement = ( specifier:Specifier, oExt: JSExt, @@ -62,6 +76,16 @@ const composeReplacement = ( ? specifier.replace(oExt, rExt) : `${specifier}${rExt}`; +/** + * Check whether the specifier has matches for a particular group of file extensions. This validates + * that the match does actually exist. + * @param parentPath The module containing the provided specifier. + * @param specifier The resolved specifier against which to check for neighbouring matches. + * @param oExt The original extension from the specifier. + * @param exts The file extensions to check. + * @returns The found match, or nothing when no definitive match is found (ex there are multiple + * matches). + */ async function checkSet( parentPath: FSAbsolutePath, specifier: Specifier, @@ -77,7 +101,7 @@ async function checkSet( } if (found.size) { - if (found.size === 1) return { replacement, isType: exts[0].startsWith('.d') }; + if (found.size === 1) return { isType: exts[0].startsWith('.d'), replacement }; logger( parentPath, diff --git a/src/resolveSpecifier.ts b/src/resolve-specifier.ts similarity index 76% rename from src/resolveSpecifier.ts rename to src/resolve-specifier.ts index ef17b93..1c945b8 100644 --- a/src/resolveSpecifier.ts +++ b/src/resolve-specifier.ts @@ -4,6 +4,11 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; import type { FSAbsolutePath, Specifier } from './index.d.ts'; +/** + * Determine the fully resolved module indicated by the specifier. + * @param parentPath The module containing the provided specifier. + * @param specifier The specifier to potentially correct. + */ export function resolveSpecifier( parentPath: FSAbsolutePath, specifier: Specifier, @@ -20,7 +25,7 @@ export function resolveSpecifier( try { resolvedSpecifierUrl = import.meta.resolve(specifier, parentUrl); } catch (err) { - console.error({ specifier, parentPath }); + if (err && typeof err === 'object') Object.assign(err, { specifier, parentPath }); throw err; } From 101ecca844cc3572f4e4bb89d328d08b292cb363 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 27 Oct 2024 14:19:40 +0100 Subject: [PATCH 7/8] fix: fexists test --- src/fexists.test.ts | 62 ++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/src/fexists.test.ts b/src/fexists.test.ts index 30cf277..f5f6eaa 100644 --- a/src/fexists.test.ts +++ b/src/fexists.test.ts @@ -1,7 +1,6 @@ import assert from 'node:assert/strict'; import { type Mock, - after, afterEach, before, describe, @@ -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; +type ResolveSpecifier = typeof import('./resolve-specifier.ts').resolveSpecifier; +// type MockModuleContext = ReturnType; const RESOLVED_SPECIFIER_ERR = 'Resolved specifier did not match expected'; @@ -22,21 +22,26 @@ describe('fexists', () => { const constants = { F_OK: null }; let mock__access: Mock['mock']; - let mock__fs: MockModuleContext; + let mock__resolveSpecifier: Mock['mock']; before(() => { const access = mock.fn(); ({ 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(); + ({ 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', () => { @@ -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, ); }); @@ -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, ); }); From d515aad38daaa7232718cb7f100e0a75b98f6702 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 27 Oct 2024 17:17:20 +0100 Subject: [PATCH 8/8] fixup!: restore `--test` flag to npm command --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 571e1a3..8dd77e0 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ }, "scripts": { "start": "node --no-warnings --experimental-import-meta-resolve --experimental-strip-types ./src/workflow.ts", - "test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --experimental-strip-types './**/*.test.ts'" + "test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --experimental-strip-types --test './**/*.test.ts'" }, "repository": { "type": "git",