From a8c61c5ed10d5102dd26fbb64d84b3c1462be6de Mon Sep 17 00:00:00 2001 From: Jonathan Date: Fri, 13 Dec 2019 16:13:29 +0100 Subject: [PATCH 1/9] Add a test case that covers the bug in #7511 --- e2e/__tests__/watchDynamicRequires.test.ts | 17 +++++++++++++++++ .../__tests__/dynamicRequire.test.js | 9 +++++++++ e2e/watch-dynamic-requires/dynamicRequire.js | 6 ++++++ e2e/watch-dynamic-requires/package.json | 7 +++++++ e2e/watch-dynamic-requires/source.js | 1 + 5 files changed, 40 insertions(+) create mode 100644 e2e/__tests__/watchDynamicRequires.test.ts create mode 100644 e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js create mode 100644 e2e/watch-dynamic-requires/dynamicRequire.js create mode 100644 e2e/watch-dynamic-requires/package.json create mode 100644 e2e/watch-dynamic-requires/source.js diff --git a/e2e/__tests__/watchDynamicRequires.test.ts b/e2e/__tests__/watchDynamicRequires.test.ts new file mode 100644 index 000000000000..f4876314e1e1 --- /dev/null +++ b/e2e/__tests__/watchDynamicRequires.test.ts @@ -0,0 +1,17 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import * as path from 'path'; +import {json as runWithJson} from '../runJest'; + +const dir = path.resolve(__dirname, '../watch-dynamic-requires'); + +test('successfully transpiles async', () => { + const {json} = runWithJson(dir, ['--findRelatedTests', 'dynamicRequire.js']); + expect(json.success).toBe(true); + expect(json.numTotalTests).toBe(2); +}); diff --git a/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js b/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js new file mode 100644 index 000000000000..e13cf232ed51 --- /dev/null +++ b/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js @@ -0,0 +1,9 @@ +test('loading a file with a dynamic local require should work', () => { + const {withStandardResolution} = require('../dynamicRequire'); + expect(withStandardResolution()).toBe(1); +}); + +test('loading a file with a dynamic require and custom resolve should work', () => { + const {withCustomResolution} = require('../dynamicRequire'); + expect(withCustomResolution()).toBe(1); +}); diff --git a/e2e/watch-dynamic-requires/dynamicRequire.js b/e2e/watch-dynamic-requires/dynamicRequire.js new file mode 100644 index 000000000000..3a41662a33a1 --- /dev/null +++ b/e2e/watch-dynamic-requires/dynamicRequire.js @@ -0,0 +1,6 @@ +const dynamicModuleName = 'source'; + +module.exports.withStandardResolution = () => + require(`./${dynamicModuleName}.js`); +module.exports.withCustomResolution = () => + require(`$asdf/${dynamicModuleName}.js`); diff --git a/e2e/watch-dynamic-requires/package.json b/e2e/watch-dynamic-requires/package.json new file mode 100644 index 000000000000..956feac1e475 --- /dev/null +++ b/e2e/watch-dynamic-requires/package.json @@ -0,0 +1,7 @@ +{ + "jest": { + "moduleNameMapper": { + "^\\$asdf/(.*)$": "/$1" + } + } +} diff --git a/e2e/watch-dynamic-requires/source.js b/e2e/watch-dynamic-requires/source.js new file mode 100644 index 000000000000..bd816eaba4ca --- /dev/null +++ b/e2e/watch-dynamic-requires/source.js @@ -0,0 +1 @@ +module.exports = 1; From 75210d5fe5a0a7597af9cffbb6e7c961f5e1db70 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Fri, 13 Dec 2019 17:53:54 +0100 Subject: [PATCH 2/9] Allow dependency resolver to ignore unknown dependencies --- packages/jest-resolve-dependencies/src/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/jest-resolve-dependencies/src/index.ts b/packages/jest-resolve-dependencies/src/index.ts index 1a6345e565c9..1278805980f1 100644 --- a/packages/jest-resolve-dependencies/src/index.ts +++ b/packages/jest-resolve-dependencies/src/index.ts @@ -58,8 +58,12 @@ class DependencyResolver { dependency, options, ); - } catch (e) { - resolvedDependency = this._resolver.getMockModule(file, dependency); + } catch (e1) { + try { + resolvedDependency = this._resolver.getMockModule(file, dependency); + } catch (e2) { + resolvedDependency = null; + } } if (resolvedDependency) { From eba1325e5e4b477c97f9f267fc866d53bcdf30bd Mon Sep 17 00:00:00 2001 From: Jonathan Date: Fri, 13 Dec 2019 18:12:13 +0100 Subject: [PATCH 3/9] Add changelog line --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 053ebb3bcbe0..42b690e5c17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ ### Fixes +- `[jest-resolve-dependencies]` Handle dynamic dependencies correctly even when using module maps ([#9303](https://github.com/facebook/jest/pull/9303)) - `[expect]` Display `expectedDiff` more carefully in `toBeCloseTo` ([#8389](https://github.com/facebook/jest/pull/8389)) - `[expect]` Avoid incorrect difference for subset when `toMatchObject` fails ([#9005](https://github.com/facebook/jest/pull/9005)) - `[expect]` Consider all RegExp flags for equality ([#9167](https://github.com/facebook/jest/pull/9167)) From 7ba301934289b300682df3b9186404baba0ef427 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Sat, 14 Dec 2019 11:42:36 +0100 Subject: [PATCH 4/9] Include the copyright header in added files --- .../__tests__/dynamicRequire.test.js | 7 +++++++ e2e/watch-dynamic-requires/dynamicRequire.js | 7 +++++++ e2e/watch-dynamic-requires/source.js | 7 +++++++ 3 files changed, 21 insertions(+) diff --git a/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js b/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js index e13cf232ed51..dc9599defc4a 100644 --- a/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js +++ b/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + test('loading a file with a dynamic local require should work', () => { const {withStandardResolution} = require('../dynamicRequire'); expect(withStandardResolution()).toBe(1); diff --git a/e2e/watch-dynamic-requires/dynamicRequire.js b/e2e/watch-dynamic-requires/dynamicRequire.js index 3a41662a33a1..c154ef61656d 100644 --- a/e2e/watch-dynamic-requires/dynamicRequire.js +++ b/e2e/watch-dynamic-requires/dynamicRequire.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + const dynamicModuleName = 'source'; module.exports.withStandardResolution = () => diff --git a/e2e/watch-dynamic-requires/source.js b/e2e/watch-dynamic-requires/source.js index bd816eaba4ca..61193c4c1d16 100644 --- a/e2e/watch-dynamic-requires/source.js +++ b/e2e/watch-dynamic-requires/source.js @@ -1 +1,8 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + module.exports = 1; From c5ec9a0b167862d20581aa8329769daa1682dbb2 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 16 Dec 2019 08:37:32 +0100 Subject: [PATCH 5/9] Remove unnecessary exception variables --- packages/jest-resolve-dependencies/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jest-resolve-dependencies/src/index.ts b/packages/jest-resolve-dependencies/src/index.ts index 1278805980f1..3f7791c8725e 100644 --- a/packages/jest-resolve-dependencies/src/index.ts +++ b/packages/jest-resolve-dependencies/src/index.ts @@ -58,10 +58,10 @@ class DependencyResolver { dependency, options, ); - } catch (e1) { + } catch { try { resolvedDependency = this._resolver.getMockModule(file, dependency); - } catch (e2) { + } catch { resolvedDependency = null; } } From ae7aae93b8d90d900c7a4af2007589d36dac4560 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 16 Dec 2019 11:20:03 +0100 Subject: [PATCH 6/9] Move changelog entry to remain in alphabetical order --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42b690e5c17e..4641b3178cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,6 @@ ### Fixes -- `[jest-resolve-dependencies]` Handle dynamic dependencies correctly even when using module maps ([#9303](https://github.com/facebook/jest/pull/9303)) - `[expect]` Display `expectedDiff` more carefully in `toBeCloseTo` ([#8389](https://github.com/facebook/jest/pull/8389)) - `[expect]` Avoid incorrect difference for subset when `toMatchObject` fails ([#9005](https://github.com/facebook/jest/pull/9005)) - `[expect]` Consider all RegExp flags for equality ([#9167](https://github.com/facebook/jest/pull/9167)) @@ -63,6 +62,7 @@ - `[jest-reporters]` Make node-notifier an optional dependency ([#8918](https://github.com/facebook/jest/pull/8918)) - `[jest-reporters]` Make all arguments to methods on `BaseReporter` optional ([#9159](https://github.com/facebook/jest/pull/9159)) - `[jest-resolve]`: Set MODULE_NOT_FOUND as error code when module is not resolved from paths ([#8487](https://github.com/facebook/jest/pull/8487)) +- `[jest-resolve-dependencies]` Handle dynamic dependencies correctly even when using module maps ([#9303](https://github.com/facebook/jest/pull/9303)) - `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859)) - `[jest-snapshot]` Distinguish empty string from external snapshot not written ([#8880](https://github.com/facebook/jest/pull/8880)) - `[jest-snapshot]` [**BREAKING**] Distinguish empty string from internal snapshot not written ([#8898](https://github.com/facebook/jest/pull/8898)) From 4fcbf1f5775aac542cc1cdbea540ee92c69a92c6 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 16 Dec 2019 12:49:18 +0100 Subject: [PATCH 7/9] Remove unnecessary null assignment --- packages/jest-resolve-dependencies/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-resolve-dependencies/src/index.ts b/packages/jest-resolve-dependencies/src/index.ts index 3f7791c8725e..6f2d377b9f30 100644 --- a/packages/jest-resolve-dependencies/src/index.ts +++ b/packages/jest-resolve-dependencies/src/index.ts @@ -62,7 +62,7 @@ class DependencyResolver { try { resolvedDependency = this._resolver.getMockModule(file, dependency); } catch { - resolvedDependency = null; + // leave resolvedDependency as undefined if nothing can be found } } From 30711a58a4ee88e4d9af06206f25a073711b4a89 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 16 Dec 2019 15:29:37 +0100 Subject: [PATCH 8/9] Rename e2e test to better match what the test actually does --- ...hDynamicRequires.test.ts => dynamicRequireDependencies.ts} | 4 ++-- .../__tests__/dynamicRequire.test.js | 0 .../dynamicRequire.js | 0 .../package.json | 0 .../source.js | 0 5 files changed, 2 insertions(+), 2 deletions(-) rename e2e/__tests__/{watchDynamicRequires.test.ts => dynamicRequireDependencies.ts} (76%) rename e2e/{watch-dynamic-requires => dynamic-require-dependencies}/__tests__/dynamicRequire.test.js (100%) rename e2e/{watch-dynamic-requires => dynamic-require-dependencies}/dynamicRequire.js (100%) rename e2e/{watch-dynamic-requires => dynamic-require-dependencies}/package.json (100%) rename e2e/{watch-dynamic-requires => dynamic-require-dependencies}/source.js (100%) diff --git a/e2e/__tests__/watchDynamicRequires.test.ts b/e2e/__tests__/dynamicRequireDependencies.ts similarity index 76% rename from e2e/__tests__/watchDynamicRequires.test.ts rename to e2e/__tests__/dynamicRequireDependencies.ts index f4876314e1e1..3f6c96a9731a 100644 --- a/e2e/__tests__/watchDynamicRequires.test.ts +++ b/e2e/__tests__/dynamicRequireDependencies.ts @@ -8,9 +8,9 @@ import * as path from 'path'; import {json as runWithJson} from '../runJest'; -const dir = path.resolve(__dirname, '../watch-dynamic-requires'); +const dir = path.resolve(__dirname, '../dynamic-require-dependencies'); -test('successfully transpiles async', () => { +test('successfully runs tests with dynamic dependencies', () => { const {json} = runWithJson(dir, ['--findRelatedTests', 'dynamicRequire.js']); expect(json.success).toBe(true); expect(json.numTotalTests).toBe(2); diff --git a/e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js b/e2e/dynamic-require-dependencies/__tests__/dynamicRequire.test.js similarity index 100% rename from e2e/watch-dynamic-requires/__tests__/dynamicRequire.test.js rename to e2e/dynamic-require-dependencies/__tests__/dynamicRequire.test.js diff --git a/e2e/watch-dynamic-requires/dynamicRequire.js b/e2e/dynamic-require-dependencies/dynamicRequire.js similarity index 100% rename from e2e/watch-dynamic-requires/dynamicRequire.js rename to e2e/dynamic-require-dependencies/dynamicRequire.js diff --git a/e2e/watch-dynamic-requires/package.json b/e2e/dynamic-require-dependencies/package.json similarity index 100% rename from e2e/watch-dynamic-requires/package.json rename to e2e/dynamic-require-dependencies/package.json diff --git a/e2e/watch-dynamic-requires/source.js b/e2e/dynamic-require-dependencies/source.js similarity index 100% rename from e2e/watch-dynamic-requires/source.js rename to e2e/dynamic-require-dependencies/source.js From 5bc89d952f5cf95f0924e18ec94ae157fee5a07d Mon Sep 17 00:00:00 2001 From: Jonathan Date: Fri, 27 Dec 2019 18:59:27 +0000 Subject: [PATCH 9/9] Add an additional unit test to cover this case --- .../src/__tests__/dependency_resolver.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.ts b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.ts index 7efd09980a9e..d0edebc0f83f 100644 --- a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.ts +++ b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import Resolver = require('jest-resolve'); import {tmpdir} from 'os'; import * as path from 'path'; import {Config} from '@jest/types'; @@ -15,6 +16,7 @@ import DependencyResolver from '../index'; const maxWorkers = 1; let dependencyResolver: DependencyResolver; +let runtimeContextResolver: Resolver; let Runtime; let config: Config.ProjectConfig; const cases: Record = { @@ -29,11 +31,13 @@ beforeEach(() => { config = makeProjectConfig({ cacheDirectory: path.resolve(tmpdir(), 'jest-resolve-dependencies-test'), moduleDirectories: ['node_modules'], + moduleNameMapper: [['^\\$asdf/(.*)$', '/$1']], rootDir: '.', roots: ['./packages/jest-resolve-dependencies'], }); return Runtime.createContext(config, {maxWorkers, watchman: false}).then( (runtimeContext: any) => { + runtimeContextResolver = runtimeContext.resolver; dependencyResolver = new DependencyResolver( runtimeContext.resolver, runtimeContext.hasteFS, @@ -106,3 +110,18 @@ test('resolves inverse dependencies from available snapshot', () => { ]), ); }); + +test('resolves dependencies correctly when dependency resolution fails', () => { + jest.spyOn(runtimeContextResolver, 'resolveModule').mockImplementation(() => { + throw new Error('resolveModule has failed'); + }); + jest.spyOn(runtimeContextResolver, 'getMockModule').mockImplementation(() => { + throw new Error('getMockModule has failed'); + }); + + const resolved = dependencyResolver.resolve( + path.resolve(__dirname, '__fixtures__', 'file.test.js'), + ); + + expect(resolved).toEqual([]); +});