Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set MODULE_NOT_FOUND code when module is not resolved from paths #8487

Merged
merged 13 commits into from
Nov 9, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

- `[babel-plugin-jest-hoist]` Expand list of whitelisted globals in global mocks ([#8429](https://github.com/facebook/jest/pull/8429)
- `[jest-core]` Make watch plugin initialization errors look nice ([#8422](https://github.com/facebook/jest/pull/8422))
- `[jest-resolve]`: Set MODULE_NOT_FOUND as error code when module is not resolved from paths ([#8487](https://github.com/facebook/jest/pull/8487))

### Chore & Maintenance

Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/moduleNameMapper.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ FAIL __tests__/index.js
12 | module.exports = () => 'test';
13 |

at createNoMappedModuleFoundError (../../packages/jest-resolve/build/index.js:472:17)
at createNoMappedModuleFoundError (../../packages/jest-resolve/build/index.js:479:17)
at Object.require (index.js:10:1)
`;
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ FAIL __tests__/test.js
| ^
4 |

at Resolver.resolveModule (../../packages/jest-resolve/build/index.js:230:17)
at Resolver.resolveModule (../../packages/jest-resolve/build/index.js:238:17)
at Object.require (index.js:3:18)
`;
6 changes: 6 additions & 0 deletions e2e/resolve-with-paths/__tests__/resolveWithPaths.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,9 @@ test('throws an error if the module cannot be found from given paths', () => {
"Cannot resolve module './mod.js' from paths ['..'] from "
);
});

test('throws module not found error if the module cannot be found from given paths', () => {
expect(() => require.resolve('./mod.js', {paths: ['..']})).toThrow(
expect.objectContaining({code: 'MODULE_NOT_FOUND'})
);
});
9 changes: 9 additions & 0 deletions e2e/resolve/__tests__/resolve.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,12 @@ test('should require resolve haste mocks correctly', () => {

expect(require('Test6').key).toBe('mock');
});

test('should throw module not found error if the module cannot be found', () => {
expect(() => require('Test7')).toThrow(
expect.objectContaining({
code: 'MODULE_NOT_FOUND',
message: "Cannot find module 'Test7' from 'resolve.test.js'",
})
);
});
10 changes: 10 additions & 0 deletions packages/jest-resolve/src/ModuleNotFoundError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +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.
*/

export default class ModuleNotFoundError extends Error {
Copy link
Collaborator

@thymikee thymikee May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Node 6 snapshot failures show pretty important change (not sure why it's different on higher versions though), since now we don't point to the place error happened, but to the error itself. We have a utility Error class called ErrorWithStack – maybe you could play with it and extend it directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like jest-resolve doesn't depend jest-utils, so can I use Error.captureStackTrace instead of extend ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I can add dependency for jest-resolve, maybe we can move ModuleNotFoundError to jest-util as well

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about that, @SimenB?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @SimenB

code = 'MODULE_NOT_FOUND';
}
4 changes: 2 additions & 2 deletions packages/jest-resolve/src/defaultResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import pnpResolver from 'jest-pnp-resolver';
import {Config} from '@jest/types';
import isBuiltinModule from './isBuiltinModule';
import nodeModulesPaths from './nodeModulesPaths';
import ModuleNotFoundError from './ModuleNotFoundError';

type ResolverOptions = {
basedir: Config.Path;
Expand Down Expand Up @@ -80,10 +81,9 @@ function resolveSync(
return target;
}

const err: Error & {code?: string} = new Error(
const err = new ModuleNotFoundError(
"Cannot find module '" + target + "' from '" + basedir + "'",
);
err.code = 'MODULE_NOT_FOUND';
throw err;

/*
Expand Down
9 changes: 7 additions & 2 deletions packages/jest-resolve/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import nodeModulesPaths from './nodeModulesPaths';
import isBuiltinModule from './isBuiltinModule';
import defaultResolver from './defaultResolver';
import {ResolverConfig} from './types';
import ModuleNotFoundError from './ModuleNotFoundError';

type FindNodeModuleConfig = {
basedir: Config.Path;
Expand Down Expand Up @@ -102,6 +103,10 @@ class Resolver {
return null;
}

static createModuleNotFoundError(message?: string) {
return new ModuleNotFoundError(message);
}

resolveModuleFromDirIfExists(
dirname: Config.Path,
moduleName: string,
Expand Down Expand Up @@ -205,10 +210,10 @@ class Resolver {
// produces an error based on the dirname but we have the actual current
// module name available.
const relativePath = path.relative(dirname, from);
const err: Error & {code?: string} = new Error(

const err = new ModuleNotFoundError(
`Cannot find module '${moduleName}' from '${relativePath || '.'}'`,
);
err.code = 'MODULE_NOT_FOUND';
throw err;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -647,11 +647,12 @@ class Runtime {
return module;
}
}
throw new Error(
const err = Resolver.createModuleNotFoundError(
clarkdo marked this conversation as resolved.
Show resolved Hide resolved
`Cannot resolve module '${moduleName}' from paths ['${paths.join(
"', '",
)}'] from ${from}`,
);
throw err;
}
try {
return this._resolveModule(from, moduleName);
Expand Down