Skip to content

Commit 9ba224d

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#26305 from ajafff/common-prefix
moduleNameResolver: fix getCommonPrefix
2 parents 19e04b2 + 4db7c86 commit 9ba224d

File tree

2 files changed

+100
-18
lines changed

2 files changed

+100
-18
lines changed

‎src/compiler/moduleNameResolver.ts‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -403,44 +403,44 @@ namespace ts {
403403

404404
const resolvedFileName = result.resolvedModule && result.resolvedModule.resolvedFileName;
405405
// find common prefix between directory and resolved file name
406-
// this common prefix should be the shorted path that has the same resolution
406+
// this common prefix should be the shortest path that has the same resolution
407407
// directory: /a/b/c/d/e
408408
// resolvedFileName: /a/b/foo.d.ts
409-
const commonPrefix = getCommonPrefix(path, resolvedFileName);
409+
// commonPrefix: /a/b
410+
// for failed lookups cache the result for every directory up to root
411+
const commonPrefix = resolvedFileName && getCommonPrefix(path, resolvedFileName);
410412
let current = path;
411-
while (true) {
413+
while (current !== commonPrefix) {
412414
const parent = getDirectoryPath(current);
413415
if (parent === current || directoryPathMap.has(parent)) {
414416
break;
415417
}
416418
directoryPathMap.set(parent, result);
417419
current = parent;
418-
419-
if (current === commonPrefix) {
420-
break;
421-
}
422420
}
423421
}
424422

425-
function getCommonPrefix(directory: Path, resolution: string | undefined) {
426-
if (resolution === undefined) {
427-
return undefined;
428-
}
423+
function getCommonPrefix(directory: Path, resolution: string) {
429424
const resolutionDirectory = toPath(getDirectoryPath(resolution), currentDirectory, getCanonicalFileName);
430425

431426
// find first position where directory and resolution differs
432427
let i = 0;
433-
while (i < Math.min(directory.length, resolutionDirectory.length) && directory.charCodeAt(i) === resolutionDirectory.charCodeAt(i)) {
428+
const limit = Math.min(directory.length, resolutionDirectory.length);
429+
while (i < limit && directory.charCodeAt(i) === resolutionDirectory.charCodeAt(i)) {
434430
i++;
435431
}
436-
437-
// find last directory separator before position i
438-
const sep = directory.lastIndexOf(directorySeparator, i);
439-
if (sep < 0) {
432+
if (i === directory.length && (resolutionDirectory.length === i || resolutionDirectory[i] === directorySeparator)) {
433+
return directory;
434+
}
435+
const rootLength = getRootLength(directory);
436+
if (i < rootLength) {
440437
return undefined;
441438
}
442-
443-
return directory.substr(0, sep);
439+
const sep = directory.lastIndexOf(directorySeparator, i - 1);
440+
if (sep === -1) {
441+
return undefined;
442+
}
443+
return directory.substr(0, Math.max(sep, rootLength));
444444
}
445445
}
446446
}

‎src/testRunner/unittests/moduleResolution.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,88 @@ namespace ts {
194194
});
195195

196196
describe("Node module resolution - non-relative paths", () => {
197+
it("computes correct commonPrefix for moduleName cache", () => {
198+
const resolutionCache = createModuleResolutionCache("/", (f) => f);
199+
let cache = resolutionCache.getOrCreateCacheForModuleName("a");
200+
cache.set("/sub", {
201+
resolvedModule: {
202+
originalPath: undefined,
203+
resolvedFileName: "/sub/node_modules/a/index.ts",
204+
isExternalLibraryImport: true,
205+
extension: Extension.Ts,
206+
},
207+
failedLookupLocations: [],
208+
});
209+
assert.isDefined(cache.get("/sub"));
210+
assert.isUndefined(cache.get("/"));
211+
212+
cache = resolutionCache.getOrCreateCacheForModuleName("b");
213+
cache.set("/sub/dir/foo", {
214+
resolvedModule: {
215+
originalPath: undefined,
216+
resolvedFileName: "/sub/directory/node_modules/b/index.ts",
217+
isExternalLibraryImport: true,
218+
extension: Extension.Ts,
219+
},
220+
failedLookupLocations: [],
221+
});
222+
assert.isDefined(cache.get("/sub/dir/foo"));
223+
assert.isDefined(cache.get("/sub/dir"));
224+
assert.isDefined(cache.get("/sub"));
225+
assert.isUndefined(cache.get("/"));
226+
227+
cache = resolutionCache.getOrCreateCacheForModuleName("c");
228+
cache.set("/foo/bar", {
229+
resolvedModule: {
230+
originalPath: undefined,
231+
resolvedFileName: "/bar/node_modules/c/index.ts",
232+
isExternalLibraryImport: true,
233+
extension: Extension.Ts,
234+
},
235+
failedLookupLocations: [],
236+
});
237+
assert.isDefined(cache.get("/foo/bar"));
238+
assert.isDefined(cache.get("/foo"));
239+
assert.isDefined(cache.get("/"));
240+
241+
cache = resolutionCache.getOrCreateCacheForModuleName("d");
242+
cache.set("/foo", {
243+
resolvedModule: {
244+
originalPath: undefined,
245+
resolvedFileName: "/foo/index.ts",
246+
isExternalLibraryImport: true,
247+
extension: Extension.Ts,
248+
},
249+
failedLookupLocations: [],
250+
});
251+
assert.isDefined(cache.get("/foo"));
252+
assert.isUndefined(cache.get("/"));
253+
254+
cache = resolutionCache.getOrCreateCacheForModuleName("e");
255+
cache.set("c:/foo", {
256+
resolvedModule: {
257+
originalPath: undefined,
258+
resolvedFileName: "d:/bar/node_modules/e/index.ts",
259+
isExternalLibraryImport: true,
260+
extension: Extension.Ts,
261+
},
262+
failedLookupLocations: [],
263+
});
264+
assert.isDefined(cache.get("c:/foo"));
265+
assert.isDefined(cache.get("c:/"));
266+
assert.isUndefined(cache.get("d:/"));
267+
268+
cache = resolutionCache.getOrCreateCacheForModuleName("f");
269+
cache.set("/foo/bar/baz", {
270+
resolvedModule: undefined,
271+
failedLookupLocations: [],
272+
});
273+
assert.isDefined(cache.get("/foo/bar/baz"));
274+
assert.isDefined(cache.get("/foo/bar"));
275+
assert.isDefined(cache.get("/foo"));
276+
assert.isDefined(cache.get("/"));
277+
});
278+
197279
it("load module as file - ts files not loaded", () => {
198280
test(/*hasDirectoryExists*/ false);
199281
test(/*hasDirectoryExists*/ true);

0 commit comments

Comments
 (0)