Skip to content

Commit a45b40b

Browse files
AugustinMauroyAugustinMauroy
committed
update from feedback
1 parent e74b736 commit a45b40b

File tree

2 files changed

+54
-135
lines changed

2 files changed

+54
-135
lines changed

‎recipes/correct-ts-specifiers/src/map-imports.test.ts‎

Lines changed: 51 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -1,174 +1,93 @@
11
import assert from 'node:assert/strict';
2-
import path from 'node:path';
3-
import { type Mock, after, afterEach, before, describe, it, mock } from 'node:test';
2+
import { type Mock, before, describe, it, mock, afterEach } from 'node:test';
43
import { fileURLToPath } from 'node:url';
54

6-
import { dExts, jsExts, suspectExts, tsExts } from './exts.ts';
5+
import { dExts } from './exts.ts';
76
import type { FSAbsolutePath } from './index.d.ts';
87

9-
type MockModuleContext = ReturnType<typeof mock.module>;
108

119
type Logger = typeof import('@nodejs/codemod-utils/logger').logger;
12-
type ReplaceJSExtWithTSExt = typeof import('./replace-js-ext-with-ts-ext.ts').replaceJSExtWithTSExt;
10+
type MapImports = typeof import('./map-imports.ts').mapImports;
1311

14-
describe('Correcting ts file extensions', { concurrency: true }, () => {
12+
describe('Map Imports', { concurrency: true }, () => {
1513
const originatingFilePath = fileURLToPath(import.meta.resolve('./test.ts')) as FSAbsolutePath;
16-
const fixturesDir = path.join(import.meta.dirname, 'fixtures/e2e') as FSAbsolutePath;
17-
const catSpecifier = path.join(fixturesDir, 'Cat.ts') as FSAbsolutePath;
18-
1914
let mock__log: Mock<Logger>['mock'];
20-
let mock__logger: MockModuleContext;
21-
let replaceJSExtWithTSExt: ReplaceJSExtWithTSExt;
15+
let mapImports: MapImports;
2216

2317
before(async () => {
2418
const logger = mock.fn<Logger>();
2519
({ mock: mock__log } = logger);
26-
mock__logger = mock.module('@nodejs/codemod-utils/logger', {
20+
mock.module('@nodejs/codemod-utils/logger', {
2721
namedExports: { logger },
2822
});
2923

30-
({ replaceJSExtWithTSExt } = await import('./replace-js-ext-with-ts-ext.ts'));
24+
({ mapImports } = await import('./map-imports.ts'));
3125
});
3226

3327
afterEach(() => {
34-
// TODO delete me
3528
mock__log.resetCalls();
3629
});
3730

38-
after(() => {
39-
// TODO delete me
40-
mock__logger.restore();
41-
});
31+
it('unambiguous: should skip a node builtin specifier', async () => {
32+
const output = await mapImports(originatingFilePath, 'node:console');
4233

43-
describe.todo('Get type def specifier from package.json');
34+
assert.equal(output.replacement, undefined);
35+
assert.notEqual(output.isType, true);
36+
});
4437

45-
describe('mapped extension exists', () => {
46-
describe('unambiguous match', () => {
47-
it('should return an updated specifier', async () => {
48-
for (const jsExt of jsExts) {
49-
const output = await replaceJSExtWithTSExt(originatingFilePath, `./fixtures/rep${jsExt}`);
38+
it('quasi-ambiguous: should append a JS extension when path resolves to a file', async () => {
39+
const specifier = './fixtures/bar';
40+
const output = await mapImports(originatingFilePath, specifier);
5041

51-
assert.equal(output.replacement, `./fixtures/rep${suspectExts[jsExt]}`);
52-
assert.equal(output.isType, false);
53-
}
54-
});
55-
});
56-
57-
describe('declaration files', () => {
58-
describe('ambiguous match', () => {
59-
it('should skip and log error', async () => {
60-
const base = './fixtures/d/ambiguous/index';
61-
const output = await replaceJSExtWithTSExt(originatingFilePath, `${base}.js`);
62-
63-
assert.equal(output.replacement, null);
64-
65-
const { 2: msg } = mock__log.calls[0].arguments;
66-
assert.match(msg, /disambiguate/);
67-
for (const dExt of dExts) assert.match(msg, new RegExp(`${base}${dExt}`));
68-
});
69-
});
70-
71-
describe('unambiguous match', () => {
72-
it('should return an updated specifier', async () => {
73-
for (const dExt of dExts) {
74-
const base = `./fixtures/d/unambiguous/${dExt.split('.').pop()}/index`;
75-
const output = await replaceJSExtWithTSExt(originatingFilePath, `${base}.js`);
76-
77-
assert.equal(output.replacement, `${base}${dExt}`);
78-
assert.equal(output.isType, true);
79-
}
80-
});
81-
82-
it.skip('should update a subpath of a node module', async () => {
83-
const specifierBase = 'types/a';
84-
const { isType, replacement } = await replaceJSExtWithTSExt(
85-
catSpecifier,
86-
`${specifierBase}.js`,
87-
);
88-
89-
assert.equal(isType, true);
90-
assert.equal(replacement, `${specifierBase}.d.ts`);
91-
});
92-
});
93-
});
42+
assert.equal(output.replacement, `${specifier}.js`);
43+
assert.notEqual(output.isType, true);
9444
});
9545

96-
describe('mapped extension does NOT exist', () => {
97-
it('should skip and log error', async () => {
98-
for (const jsExt of jsExts) {
99-
const output = await replaceJSExtWithTSExt(originatingFilePath, `./fixtures/skip${jsExt}`);
46+
it('quasi-ambiguous: should append a TS extension when path resolves to a file', async () => {
47+
const specifier = './fixtures/foo';
48+
const output = await mapImports(originatingFilePath, specifier);
10049

101-
assert.equal(output.replacement, null);
102-
assert.equal(output.isType, undefined);
103-
}
104-
});
50+
assert.equal(output.replacement, `${specifier}.ts`);
51+
assert.notEqual(output.isType, true);
10552
});
10653

107-
describe('specifier is inherently a directory', () => {
108-
it('should attempt to find an index file', async () => {
109-
for (const base of ['.', './']) {
110-
for (const jsExt of jsExts) {
111-
const output = await replaceJSExtWithTSExt(
112-
fileURLToPath(
113-
import.meta.resolve(`./fixtures/dir/${jsExt.slice(1)}/test.ts`),
114-
) as FSAbsolutePath,
115-
base,
116-
);
117-
118-
assert.equal(output.replacement, `${base}${base.endsWith('/') ? '' : '/'}index${jsExt}`);
119-
assert.equal(output.isType, false);
120-
}
121-
122-
for (const tsExt of tsExts) {
123-
const output = await replaceJSExtWithTSExt(
124-
fileURLToPath(
125-
import.meta.resolve(`./fixtures/dir/${tsExt.slice(1)}/test.ts`),
126-
) as FSAbsolutePath,
127-
base,
128-
);
129-
130-
assert.equal(output.replacement, `${base}${base.endsWith('/') ? '' : '/'}index${tsExt}`);
131-
assert.equal(output.isType, false);
132-
}
133-
}
134-
});
135-
});
54+
it('unambiguous: should replace ".js" → ".ts" when JS file does NOT exist & TS file DOES exist', async () => {
55+
const specifier = './fixtures/noexist.js';
56+
const output = await mapImports(originatingFilePath, specifier);
13657

137-
describe('specifier is NOT inherently a directory', () => {
138-
it('should attempt to find an index file', async () => {
139-
for (const dExt of dExts) {
140-
const base = `./fixtures/d/unambiguous/${dExt.slice(3)}`;
141-
const output = await replaceJSExtWithTSExt(originatingFilePath, base);
58+
assert.equal(output.replacement, undefined);
59+
assert.notEqual(output.isType, true);
14260

143-
assert.equal(output.replacement, `${base}/index${dExt}`);
144-
assert.equal(output.isType, true);
145-
}
61+
const { 0: sourcePath, 2: msg } = mock__log.calls[0].arguments;
14662

147-
for (const jsExt of jsExts) {
148-
const base = `./fixtures/dir/${jsExt.slice(1)}`;
149-
const output = await replaceJSExtWithTSExt(originatingFilePath, base);
63+
assert.match(sourcePath, new RegExp(originatingFilePath));
64+
assert.match(msg, /no match/i);
65+
assert.match(msg, new RegExp(specifier));
66+
});
15067

151-
assert.equal(output.replacement, `${base}/index${jsExt}`);
152-
assert.equal(output.isType, false);
153-
}
68+
it('unambiguous: should not change the file extension when JS file DOES exist & TS file does NOT exist', async () => {
69+
const specifier = './fixtures/bar.js';
70+
const output = await mapImports(originatingFilePath, specifier);
15471

155-
for (const tsExt of tsExts) {
156-
const base = `./fixtures/dir/${tsExt.slice(1)}`;
157-
const output = await replaceJSExtWithTSExt(originatingFilePath, base);
72+
assert.equal(output.replacement, undefined);
73+
assert.notEqual(output.isType, true);
74+
});
15875

159-
assert.equal(output.replacement, `${base}/index${tsExt}`);
160-
assert.equal(output.isType, false);
161-
}
162-
});
76+
it('unambiguous: should replace ".js" → ".d…" when JS file does NOT exist & a declaration file exists', async () => {
77+
for (const dExt of dExts) {
78+
const extType = dExt.split('.').pop();
79+
const specifierBase = `./fixtures/d/unambiguous/${extType}/index`;
80+
const output = await mapImports(originatingFilePath, `${specifierBase}.js`);
81+
82+
assert.equal(output.replacement, `${specifierBase}${dExt}`);
83+
assert.equal(output.isType, true);
84+
}
16385
});
16486

165-
describe('specifier contains a directory with a file extension', () => {
166-
it('should replace only the file extension', async () => {
167-
const originalSpecifier = './fixtures/e2e/qux.js';
168-
const { isType, replacement } = await replaceJSExtWithTSExt(originatingFilePath, originalSpecifier);
87+
it('ambiguous: should log and skip when both a JS & a TS file exist with the same name', async () => {
88+
const output = await mapImports(originatingFilePath, './fixtures/foo.js');
16989

170-
assert.equal(isType, false);
171-
assert.equal(replacement, `${originalSpecifier}/index.ts`);
172-
});
90+
assert.equal(output.replacement, './fixtures/foo.ts');
91+
assert.notEqual(output.isType, true);
17392
});
17493
});

‎utils/src/snapshots.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
import { basename, dirname, extname, join } from 'node:path';
22
import { snapshot } from 'node:test';
33

4-
snapshot.setResolveSnapshotPath(generateSnapshotPath);
5-
64
type SnapshotPath = Parameters<Parameters<typeof snapshot.setResolveSnapshotPath>[0]>[0];
75

86
/**
97
* @param {string} testFilePath `'/tmp/foo.test.js'`
108
* @returns `'/tmp/foo.test.snap.cjs'`
119
*/
12-
function generateSnapshotPath(testFilePath: SnapshotPath) {
10+
const generateSnapshotPath: Parameters<typeof snapshot.setResolveSnapshotPath>[0] = (testFilePath: SnapshotPath) => {
1311
if (!testFilePath) return '';
1412

1513
const ext = extname(testFilePath);
@@ -18,3 +16,5 @@ function generateSnapshotPath(testFilePath: SnapshotPath) {
1816

1917
return join(base, `${filename}.snap.cjs`);
2018
}
19+
20+
snapshot.setResolveSnapshotPath(generateSnapshotPath);

0 commit comments

Comments
 (0)