From bed93b48f561a271162ec95ddcd88deaf0e8c759 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 8 Aug 2018 11:12:48 +0200 Subject: [PATCH 1/5] fix moduleNameResolver cache Fixes: #26271 --- src/compiler/moduleNameResolver.ts | 3 +- src/testRunner/unittests/moduleResolution.ts | 21 ++++++++++++++ ...lLinkDeclarationEmitModuleNames.errors.txt | 28 +++++++++++++++++++ ...symbolLinkDeclarationEmitModuleNames.types | 6 ++-- 4 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index f93f2d7eb64..bfc3718490d 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -401,7 +401,8 @@ namespace ts { } directoryPathMap.set(path, result); - const resolvedFileName = result.resolvedModule && result.resolvedModule.resolvedFileName; + const resolvedFileName = result.resolvedModule && + (result.resolvedModule.originalPath || result.resolvedModule.resolvedFileName); // find common prefix between directory and resolved file name // this common prefix should be the shorted path that has the same resolution // directory: /a/b/c/d/e diff --git a/src/testRunner/unittests/moduleResolution.ts b/src/testRunner/unittests/moduleResolution.ts index e97a3f5fc42..84975750ac8 100644 --- a/src/testRunner/unittests/moduleResolution.ts +++ b/src/testRunner/unittests/moduleResolution.ts @@ -321,6 +321,27 @@ namespace ts { checkResolvedModule(resolution.resolvedModule, createResolvedModule(resolvedFileName, /*isExternalLibraryImport*/ true)); }); } + + it("uses originalPath for caching", () => { + const host = createModuleResolutionHost( + /*hasDirectoryExists*/ true, + { + name: "/a.ts", + symlinks: ["/sub/node_modules/a/index.ts"], + }, + ); + const cache = createModuleResolutionCache("/", (f) => f); + let resolution = nodeModuleNameResolver("a", "/sub/foo.ts", {}, host, cache); + checkResolvedModule(resolution.resolvedModule, { + extension: Extension.Ts, + isExternalLibraryImport: true, + originalPath: "/sub/node_modules/a/index.ts", + packageId: undefined, + resolvedFileName: "/a.ts", + }); + resolution = nodeModuleNameResolver("a", "/foo.ts", {}, host, cache); + assert.isUndefined(resolution.resolvedModule, "lookup in parent directory doesn't hit the cache"); + }); }); describe("Module resolution - relative imports", () => { diff --git a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt new file mode 100644 index 00000000000..42dfebba6cb --- /dev/null +++ b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt @@ -0,0 +1,28 @@ +tests/cases/compiler/monorepo/context/src/bindingkey.ts(1,29): error TS2307: Cannot find module '@loopback/context'. + + +==== tests/cases/compiler/monorepo/core/src/application.ts (0 errors) ==== + import { Constructor } from "@loopback/context"; + export type ControllerClass = Constructor; +==== tests/cases/compiler/monorepo/core/src/usage.ts (0 errors) ==== + import { ControllerClass } from './application'; + import { BindingKey } from '@loopback/context'; + + export const CONTROLLER_CLASS = BindingKey.create(null as any); // line in question +==== tests/cases/compiler/monorepo/context/src/value-promise.ts (0 errors) ==== + export type Constructor = (...args: any[]) => T; +==== tests/cases/compiler/monorepo/context/src/bindingkey.ts (1 errors) ==== + import { Constructor } from "@loopback/context" + ~~~~~~~~~~~~~~~~~~~ +!!! error TS2307: Cannot find module '@loopback/context'. + export class BindingKey { + readonly __type: T; + static create>(ctor: T) { + return new BindingKey(); + } + } + +==== tests/cases/compiler/monorepo/context/index.ts (0 errors) ==== + export * from "./src/value-promise"; + export * from "./src/bindingkey"; + \ No newline at end of file diff --git a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types index 1101327b283..b6934a8d75f 100644 --- a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types +++ b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types @@ -15,9 +15,9 @@ import { BindingKey } from '@loopback/context'; export const CONTROLLER_CLASS = BindingKey.create(null as any); // line in question >CONTROLLER_CLASS : BindingKey> >BindingKey.create(null as any) : BindingKey> ->BindingKey.create : >(ctor: T) => BindingKey +>BindingKey.create : (ctor: T) => BindingKey >BindingKey : typeof BindingKey ->create : >(ctor: T) => BindingKey +>create : (ctor: T) => BindingKey >null as any : any >null : null @@ -37,7 +37,7 @@ export class BindingKey { >__type : T static create>(ctor: T) { ->create : >(ctor: T) => BindingKey +>create : (ctor: T) => BindingKey >ctor : T return new BindingKey(); From 47e77901fd205295d0d63a54ad0841bf6786f311 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 8 Aug 2018 15:39:07 +0200 Subject: [PATCH 2/5] actually test the cache --- src/testRunner/unittests/moduleResolution.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/testRunner/unittests/moduleResolution.ts b/src/testRunner/unittests/moduleResolution.ts index 84975750ac8..95093ae88ec 100644 --- a/src/testRunner/unittests/moduleResolution.ts +++ b/src/testRunner/unittests/moduleResolution.ts @@ -329,9 +329,14 @@ namespace ts { name: "/a.ts", symlinks: ["/sub/node_modules/a/index.ts"], }, + { + name: "/sub/node_modules/a/package.json", + content: '{"version": "0.0.0", "main": "./index"}' + } ); + const compilerOptions: CompilerOptions = { moduleResolution: ModuleResolutionKind.NodeJs }; const cache = createModuleResolutionCache("/", (f) => f); - let resolution = nodeModuleNameResolver("a", "/sub/foo.ts", {}, host, cache); + let resolution = resolveModuleName("a", "/sub/foo.ts", compilerOptions, host, cache); checkResolvedModule(resolution.resolvedModule, { extension: Extension.Ts, isExternalLibraryImport: true, @@ -339,7 +344,7 @@ namespace ts { packageId: undefined, resolvedFileName: "/a.ts", }); - resolution = nodeModuleNameResolver("a", "/foo.ts", {}, host, cache); + resolution = resolveModuleName("a", "/foo.ts", compilerOptions, host, cache); assert.isUndefined(resolution.resolvedModule, "lookup in parent directory doesn't hit the cache"); }); }); From f31b9d3891e10fecc3c2526b1339d96aec8f0c61 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 8 Aug 2018 16:38:42 +0200 Subject: [PATCH 3/5] fix test --- src/testRunner/unittests/moduleResolution.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/testRunner/unittests/moduleResolution.ts b/src/testRunner/unittests/moduleResolution.ts index 95093ae88ec..f0a1ad3face 100644 --- a/src/testRunner/unittests/moduleResolution.ts +++ b/src/testRunner/unittests/moduleResolution.ts @@ -326,7 +326,7 @@ namespace ts { const host = createModuleResolutionHost( /*hasDirectoryExists*/ true, { - name: "/a.ts", + name: "/modules/a.ts", symlinks: ["/sub/node_modules/a/index.ts"], }, { @@ -342,7 +342,7 @@ namespace ts { isExternalLibraryImport: true, originalPath: "/sub/node_modules/a/index.ts", packageId: undefined, - resolvedFileName: "/a.ts", + resolvedFileName: "/modules/a.ts", }); resolution = resolveModuleName("a", "/foo.ts", compilerOptions, host, cache); assert.isUndefined(resolution.resolvedModule, "lookup in parent directory doesn't hit the cache"); From ec1ff29a8cb9c125dae96d5c8c1def471a6cbf3b Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 8 Aug 2018 17:10:22 +0200 Subject: [PATCH 4/5] really, really fix test(?) --- src/testRunner/unittests/moduleResolution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/testRunner/unittests/moduleResolution.ts b/src/testRunner/unittests/moduleResolution.ts index f0a1ad3face..155a03b1fb6 100644 --- a/src/testRunner/unittests/moduleResolution.ts +++ b/src/testRunner/unittests/moduleResolution.ts @@ -336,7 +336,7 @@ namespace ts { ); const compilerOptions: CompilerOptions = { moduleResolution: ModuleResolutionKind.NodeJs }; const cache = createModuleResolutionCache("/", (f) => f); - let resolution = resolveModuleName("a", "/sub/foo.ts", compilerOptions, host, cache); + let resolution = resolveModuleName("a", "/sub/dir/foo.ts", compilerOptions, host, cache); checkResolvedModule(resolution.resolvedModule, { extension: Extension.Ts, isExternalLibraryImport: true, From 3469b62be6131df70af255d81fd97540222681d0 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 10 Aug 2018 22:25:27 +0200 Subject: [PATCH 5/5] review comments --- src/testRunner/unittests/moduleResolution.ts | 12 ++++---- ...lLinkDeclarationEmitModuleNames.errors.txt | 28 ------------------- ...symbolLinkDeclarationEmitModuleNames.types | 6 ++-- .../symbolLinkDeclarationEmitModuleNames.ts | 2 +- 4 files changed, 9 insertions(+), 39 deletions(-) delete mode 100644 tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt diff --git a/src/testRunner/unittests/moduleResolution.ts b/src/testRunner/unittests/moduleResolution.ts index 155a03b1fb6..acaab1ef420 100644 --- a/src/testRunner/unittests/moduleResolution.ts +++ b/src/testRunner/unittests/moduleResolution.ts @@ -337,13 +337,11 @@ namespace ts { const compilerOptions: CompilerOptions = { moduleResolution: ModuleResolutionKind.NodeJs }; const cache = createModuleResolutionCache("/", (f) => f); let resolution = resolveModuleName("a", "/sub/dir/foo.ts", compilerOptions, host, cache); - checkResolvedModule(resolution.resolvedModule, { - extension: Extension.Ts, - isExternalLibraryImport: true, - originalPath: "/sub/node_modules/a/index.ts", - packageId: undefined, - resolvedFileName: "/modules/a.ts", - }); + checkResolvedModule(resolution.resolvedModule, createResolvedModule("/modules/a.ts", /*isExternalLibraryImport*/ true)); + + resolution = resolveModuleName("a", "/sub/foo.ts", compilerOptions, host, cache); + checkResolvedModule(resolution.resolvedModule, createResolvedModule("/modules/a.ts", /*isExternalLibraryImport*/ true)); + resolution = resolveModuleName("a", "/foo.ts", compilerOptions, host, cache); assert.isUndefined(resolution.resolvedModule, "lookup in parent directory doesn't hit the cache"); }); diff --git a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt deleted file mode 100644 index 42dfebba6cb..00000000000 --- a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.errors.txt +++ /dev/null @@ -1,28 +0,0 @@ -tests/cases/compiler/monorepo/context/src/bindingkey.ts(1,29): error TS2307: Cannot find module '@loopback/context'. - - -==== tests/cases/compiler/monorepo/core/src/application.ts (0 errors) ==== - import { Constructor } from "@loopback/context"; - export type ControllerClass = Constructor; -==== tests/cases/compiler/monorepo/core/src/usage.ts (0 errors) ==== - import { ControllerClass } from './application'; - import { BindingKey } from '@loopback/context'; - - export const CONTROLLER_CLASS = BindingKey.create(null as any); // line in question -==== tests/cases/compiler/monorepo/context/src/value-promise.ts (0 errors) ==== - export type Constructor = (...args: any[]) => T; -==== tests/cases/compiler/monorepo/context/src/bindingkey.ts (1 errors) ==== - import { Constructor } from "@loopback/context" - ~~~~~~~~~~~~~~~~~~~ -!!! error TS2307: Cannot find module '@loopback/context'. - export class BindingKey { - readonly __type: T; - static create>(ctor: T) { - return new BindingKey(); - } - } - -==== tests/cases/compiler/monorepo/context/index.ts (0 errors) ==== - export * from "./src/value-promise"; - export * from "./src/bindingkey"; - \ No newline at end of file diff --git a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types index b6934a8d75f..1101327b283 100644 --- a/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types +++ b/tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.types @@ -15,9 +15,9 @@ import { BindingKey } from '@loopback/context'; export const CONTROLLER_CLASS = BindingKey.create(null as any); // line in question >CONTROLLER_CLASS : BindingKey> >BindingKey.create(null as any) : BindingKey> ->BindingKey.create : (ctor: T) => BindingKey +>BindingKey.create : >(ctor: T) => BindingKey >BindingKey : typeof BindingKey ->create : (ctor: T) => BindingKey +>create : >(ctor: T) => BindingKey >null as any : any >null : null @@ -37,7 +37,7 @@ export class BindingKey { >__type : T static create>(ctor: T) { ->create : (ctor: T) => BindingKey +>create : >(ctor: T) => BindingKey >ctor : T return new BindingKey(); diff --git a/tests/cases/compiler/symbolLinkDeclarationEmitModuleNames.ts b/tests/cases/compiler/symbolLinkDeclarationEmitModuleNames.ts index 95defd4ecc7..90a5c19196e 100644 --- a/tests/cases/compiler/symbolLinkDeclarationEmitModuleNames.ts +++ b/tests/cases/compiler/symbolLinkDeclarationEmitModuleNames.ts @@ -22,4 +22,4 @@ export class BindingKey { export * from "./src/value-promise"; export * from "./src/bindingkey"; -// @link: tests/cases/compiler/monorepo/context -> tests/cases/compiler/monorepo/core/node_modules/@loopback/context \ No newline at end of file +// @link: tests/cases/compiler/monorepo/context -> tests/cases/compiler/monorepo/node_modules/@loopback/context \ No newline at end of file