Skip to content

Commit

Permalink
Throw explicit errors for common moduleFileExtension failures (#7160)
Browse files Browse the repository at this point in the history
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->

<!-- Please remember to update CHANGELOG.md in the root of the project if you have not done so. -->

## Summary

Inspired by #7158, I finally decided to throw a better error in the cases where people have custom `moduleFileExtension` resulting in more or less obscure errors.

I've tackled 2 different issues here (in separate commits, happy to split them up into separate PRs if you want).

1. If you require a file without the file extension, we try to look for files matching _with_ a file extension and list them out. Also tell the user to either include file extension in the `require` or update `moduleFileExtension`.
2. If `js` is missing from `moduleFileExtension`, jest is unable to inject into the runtime. I decided to throw an explicit configuration error rather than fixing `jest-jasmine`'s `require`, as we'd also need all of our dependencies to do the same (e.g. `source-map` throws if we do `moduleFileExtension: []` now).

Fixes #4025.

<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->

## Test plan
Integration tests added

<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->
  • Loading branch information
SimenB committed Oct 15, 2018
1 parent b55a980 commit 3157c8a
Show file tree
Hide file tree
Showing 18 changed files with 2,228 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@
- `[jest-haste-map]` [**BREAKING**] Remove support for `@providesModule` ([#6104](https://github.com/facebook/jest/pull/6104))
- `[pretty-format]` Support HTMLCollection and NodeList in DOMCollection plugin ([#7125](https://github.com/facebook/jest/pull/7125))
- `[jest-runtime]` Pass the normalized configuration to script transformers ([#7148](https://github.com/facebook/jest/pull/7148))
- `[jest-runtime]` If `require` fails without a file extension, print all files that match with one ([#7160](https://github.com/facebook/jest/pull/7160))

### Fixes

Expand All @@ -37,6 +38,7 @@
- `[jest-cli]` Fix unhandled error when a bad revision is provided to `changedSince` ([#7115](https://github.com/facebook/jest/pull/7115))
- `[jest-config]` Moved dynamically assigned `cwd` from `jest-cli` to default configuration in `jest-config` ([#7146](https://github.com/facebook/jest/pull/7146))
- `[jest-config]` Fix `getMaxWorkers` on termux ([#7154](https://github.com/facebook/jest/pull/7154))
- `[jest-runtime]` Throw an explicit error if `js` is missing from `moduleFileExtensions` ([#7160](https://github.com/facebook/jest/pull/7160))

### Chore & Maintenance

Expand Down
@@ -0,0 +1,38 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`show error message when no js moduleFileExtensions 1`] = `
"● Validation Error:
moduleFileExtensions must include 'js':
but instead received:
[\\"jsx\\"]
Please change your configuration to include 'js'.
Configuration Documentation:
https://jestjs.io/docs/configuration.html
"
`;

exports[`show error message with matching files 1`] = `
"FAIL __tests__/test.js
● Test suite failed to run
Cannot find module './some-json-file' from 'index.js'
However, Jest was able to find:
'./some-json-file.json'
You might want to include a file extension in your import, or update your 'moduleFileExtensions', which is currently ['js'].
See https://jestjs.io/docs/en/configuration#modulefileextensions-array-string
> 1 | module.exports = require('./some-json-file');
| ^
2 |
at packages/jest-resolve/build/index.js:221:17
at index.js:1:18
"
`;
53 changes: 53 additions & 0 deletions e2e/__tests__/resolve_no_file_extensions.test.js
@@ -0,0 +1,53 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. 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.
*
* @flow
*/
'use strict';

import path from 'path';
import runJest from '../runJest';
import {cleanup, extractSummary, writeFiles} from '../Utils';

const DIR = path.resolve(__dirname, '../resolve_no_extensions-no-js');

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

test('show error message with matching files', () => {
const {status, stderr} = runJest('resolve_no_extensions');
const {rest} = extractSummary(stderr);

expect(status).toBe(1);
expect(rest).toMatchSnapshot();
});

test('show error message when no js moduleFileExtensions', () => {
writeFiles(DIR, {
'index.jsx': `
module.exports ={found: true};
`,
'package.json': `
{
"jest": {
"moduleFileExtensions": ["jsx"]
}
}
`,
'test.jsx': `
const m = require('../');
test('some test', () => {
expect(m.found).toBe(true);
});
`,
});

const {status, stderr} = runJest('resolve_no_extensions-no-js');

expect(status).toBe(1);
expect(stderr).toMatchSnapshot();
});
5 changes: 5 additions & 0 deletions e2e/resolve_no_extensions/__tests__/test.js
@@ -0,0 +1,5 @@
const m = require('../');

test('some test', () => {
expect(m.found).toBe(true);
});
1 change: 1 addition & 0 deletions e2e/resolve_no_extensions/index.js
@@ -0,0 +1 @@
module.exports = require('./some-json-file');
7 changes: 7 additions & 0 deletions e2e/resolve_no_extensions/package.json
@@ -0,0 +1,7 @@
{
"jest": {
"moduleFileExtensions": [
"js"
]
}
}
3 changes: 3 additions & 0 deletions e2e/resolve_no_extensions/some-json-file.json
@@ -0,0 +1,3 @@
{
"found": true
}
87 changes: 87 additions & 0 deletions flow-typed/npm/glob_v7.1.x.js
@@ -0,0 +1,87 @@
// flow-typed signature: 7c09aef8ac07163d6ef9e3f50c6bc35c
// flow-typed version: a12a42a747/glob_v7.1.x/flow_>=v0.42.x

declare module "glob" {
declare type MinimatchOptions = {|
debug?: boolean,
nobrace?: boolean,
noglobstar?: boolean,
dot?: boolean,
noext?: boolean,
nocase?: boolean,
nonull?: boolean,
matchBase?: boolean,
nocomment?: boolean,
nonegate?: boolean,
flipNegate?: boolean
|};

declare type Options = {|
...MinimatchOptions,
cwd?: string,
root?: string,
nomount?: boolean,
mark?: boolean,
nosort?: boolean,
stat?: boolean,
silent?: boolean,
strict?: boolean,
cache?: {
[path: string]: boolean | "DIR" | "FILE" | $ReadOnlyArray<string>
},
statCache?: {
[path: string]: boolean | { isDirectory(): boolean } | void
},
symlinks?: { [path: string]: boolean | void },
realpathCache?: { [path: string]: string },
sync?: boolean,
nounique?: boolean,
nodir?: boolean,
ignore?: string | $ReadOnlyArray<string>,
follow?: boolean,
realpath?: boolean,
absolute?: boolean
|};

/**
* Called when an error occurs, or matches are found
* err
* matches: filenames found matching the pattern
*/
declare type CallBack = (err: ?Error, matches: Array<string>) => void;

declare class Glob extends events$EventEmitter {
constructor(pattern: string): this;
constructor(pattern: string, callback: CallBack): this;
constructor(pattern: string, options: Options, callback: CallBack): this;

minimatch: {};
options: Options;
aborted: boolean;
cache: {
[path: string]: boolean | "DIR" | "FILE" | $ReadOnlyArray<string>
};
statCache: {
[path: string]: boolean | { isDirectory(): boolean } | void
};
symlinks: { [path: string]: boolean | void };
realpathCache: { [path: string]: string };
found: Array<string>;

pause(): void;
resume(): void;
abort(): void;
}

declare class GlobModule {
Glob: Class<Glob>;

(pattern: string, callback: CallBack): void;
(pattern: string, options: Options, callback: CallBack): void;

hasMagic(pattern: string, options?: Options): boolean;
sync(pattern: string, options?: Options): Array<string>;
}

declare module.exports: GlobModule;
}
14 changes: 10 additions & 4 deletions packages/jest-cli/src/__tests__/SearchSource.test.js
Expand Up @@ -260,7 +260,7 @@ describe('SearchSource', () => {
it('finds tests with similar but custom file extensions', () => {
const {options: config} = normalize(
{
moduleFileExtensions: ['jsx'],
moduleFileExtensions: ['js', 'jsx'],
name,
rootDir,
testMatch,
Expand All @@ -271,14 +271,17 @@ describe('SearchSource', () => {
const relPaths = toPaths(data.tests).map(absPath =>
path.relative(rootDir, absPath),
);
expect(relPaths).toEqual([path.normalize('__testtests__/test.jsx')]);
expect(relPaths.sort()).toEqual([
path.normalize('__testtests__/test.js'),
path.normalize('__testtests__/test.jsx'),
]);
});
});

it('finds tests with totally custom foobar file extensions', () => {
const {options: config} = normalize(
{
moduleFileExtensions: ['foobar'],
moduleFileExtensions: ['js', 'foobar'],
name,
rootDir,
testMatch,
Expand All @@ -289,7 +292,10 @@ describe('SearchSource', () => {
const relPaths = toPaths(data.tests).map(absPath =>
path.relative(rootDir, absPath),
);
expect(relPaths).toEqual([path.normalize('__testtests__/test.foobar')]);
expect(relPaths.sort()).toEqual([
path.normalize('__testtests__/test.foobar'),
path.normalize('__testtests__/test.js'),
]);
});
});

Expand Down
20 changes: 20 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.js
Expand Up @@ -1273,3 +1273,23 @@ describe('testPathPattern', () => {
expect(options.onlyChanged).toBe(false);
});
});

describe('moduleFileExtensions', () => {
it('defaults to something useful', () => {
const {options} = normalize({rootDir: '/root'}, {});

expect(options.moduleFileExtensions).toEqual(['js', 'json', 'jsx', 'node']);
});

it('throws if missing `js`', () => {
expect(() =>
normalize(
{
rootDir: '/root/',
moduleFileExtensions: ['json', 'jsx'],
},
{},
),
).toThrowError("moduleFileExtensions must include 'js'");
});
});
28 changes: 27 additions & 1 deletion packages/jest-config/src/normalize.js
Expand Up @@ -36,6 +36,7 @@ import DEFAULT_CONFIG from './Defaults';
import DEPRECATED_CONFIG from './Deprecated';
import setFromArgv from './setFromArgv';
import VALID_CONFIG from './ValidConfig';

const ERROR = `${BULLET}Validation Error`;
const PRESET_EXTENSIONS = ['.json', '.js'];
const PRESET_NAME = 'jest-preset';
Expand Down Expand Up @@ -548,6 +549,32 @@ export default function normalize(options: InitialOptions, argv: Argv) {
case 'testRegex':
value = options[key] && replacePathSepForRegex(options[key]);
break;
case 'moduleFileExtensions': {
value = options[key];

// If it's the wrong type, it can throw at a later time
if (Array.isArray(value) && !value.includes('js')) {
const errorMessage =
` moduleFileExtensions must include 'js':\n` +
` but instead received:\n` +
` ${chalk.bold.red(JSON.stringify(value))}`;

// If `js` is not included, any dependency Jest itself injects into
// the environment, like jasmine or sourcemap-support, will need to
// `require` its modules with a file extension. This is not plausible
// in the long run, so it's way easier to just fail hard early.
// We might consider throwing if `json` is missing as well, as it's a
// fair assumption from modules that they can do
// `require('some-package/package') without the trailing `.json` as it
// works in Node normally.
throw createConfigError(
errorMessage +
"\n Please change your configuration to include 'js'.",
);
}

break;
}
case 'automock':
case 'bail':
case 'browser':
Expand All @@ -571,7 +598,6 @@ export default function normalize(options: InitialOptions, argv: Argv) {
case 'listTests':
case 'logHeapUsage':
case 'mapCoverage':
case 'moduleFileExtensions':
case 'name':
case 'noStackTrace':
case 'notify':
Expand Down
1 change: 1 addition & 0 deletions packages/jest-runtime/package.json
Expand Up @@ -14,6 +14,7 @@
"convert-source-map": "^1.4.0",
"exit": "^0.1.2",
"fast-json-stable-stringify": "^2.0.0",
"glob": "^7.1.3",
"graceful-fs": "^4.1.11",
"jest-config": "^23.6.0",
"jest-haste-map": "^23.6.0",
Expand Down
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Runtime requireModule with no extension throws error pointing out file with extension 1`] = `
"Cannot find module 'RegularModuleWithWrongExt' from 'root.js'
However, Jest was able to find:
'./RegularModuleWithWrongExt.txt'
You might want to include a file extension in your import, or update your 'moduleFileExtensions', which is currently ['js', 'json', 'jsx', 'node'].
See https://jestjs.io/docs/en/configuration#modulefileextensions-array-string"
`;
@@ -0,0 +1,28 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. 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.
*
*/

'use strict';

let createRuntime;

describe('Runtime requireModule with no extension', () => {
beforeEach(() => {
createRuntime = require('createRuntime');
});

it('throws error pointing out file with extension', async () => {
const runtime = await createRuntime(__filename);

expect(() =>
runtime.requireModuleOrMock(
runtime.__mockRootPath,
'RegularModuleWithWrongExt',
),
).toThrowErrorMatchingSnapshot();
});
});
@@ -0,0 +1 @@
some file

0 comments on commit 3157c8a

Please sign in to comment.