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: hoist imports of @jest/globals correctly #9806

Merged
merged 23 commits into from
Apr 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions e2e/__tests__/babelPluginJestHoist.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ beforeEach(() => {
run('yarn', DIR);
});

it('sucessfully runs the tests inside `babel-plugin-jest-hoist/`', () => {
it('successfully runs the tests inside `babel-plugin-jest-hoist/`', () => {
const {json} = runWithJson(DIR, ['--no-cache', '--coverage']);
expect(json.success).toBe(true);
expect(json.numTotalTestSuites).toBe(3);
expect(json.numTotalTestSuites).toBe(4);
});
53 changes: 53 additions & 0 deletions e2e/babel-plugin-jest-hoist/__tests__/importJest.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* 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.
*
*/

/* eslint-disable import/no-duplicates */
import {jest} from '@jest/globals';
jeysal marked this conversation as resolved.
Show resolved Hide resolved
import {jest as aliasedJest} from '@jest/globals';
import * as JestGlobals from '@jest/globals';
/* eslint-enable import/no-duplicates */

import a from '../__test_modules__/a';
import b from '../__test_modules__/b';
import c from '../__test_modules__/c';
import d from '../__test_modules__/d';

// These will be hoisted above imports

jest.unmock('../__test_modules__/a');
aliasedJest.unmock('../__test_modules__/b');
JestGlobals.jest.unmock('../__test_modules__/c');

// These will not be hoisted above imports

{
const jest = {unmock: () => {}};
jest.unmock('../__test_modules__/d');
}

// tests

test('named import', () => {
expect(a._isMockFunction).toBe(undefined);
expect(a()).toBe('unmocked');
});

test('aliased named import', () => {
expect(b._isMockFunction).toBe(undefined);
expect(b()).toBe('unmocked');
});

test('namespace import', () => {
expect(c._isMockFunction).toBe(undefined);
expect(c()).toBe('unmocked');
});

test('fake jest, shadowed import', () => {
expect(d._isMockFunction).toBe(true);
expect(d()).toBe(undefined);
});
31 changes: 24 additions & 7 deletions e2e/babel-plugin-jest-hoist/__tests__/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import a from '../__test_modules__/a';
import b from '../__test_modules__/b';
import c from '../__test_modules__/c';
import d from '../__test_modules__/d';
import e from '../__test_modules__/e';
import f from '../__test_modules__/f';
jeysal marked this conversation as resolved.
Show resolved Hide resolved
import jestBackticks from '../__test_modules__/jestBackticks';

// The virtual mock call below will be hoisted above this `require` call.
Expand All @@ -25,7 +25,16 @@ const virtualModule = require('virtual-module');
jest.unmock('react');
jest.deepUnmock('../__test_modules__/Unmocked');
jest.unmock('../__test_modules__/c').unmock('../__test_modules__/d');
jest.mock('../__test_modules__/e', () => {

let e;
(function () {
const _getJestObj = 42;
e = require('../__test_modules__/e').default;
// hoisted to the top of the function scope
jest.unmock('../__test_modules__/e');
})();

jest.mock('../__test_modules__/f', () => {
if (!global.CALLS) {
global.CALLS = 0;
}
Expand All @@ -52,8 +61,13 @@ jest.mock('has-flow-types', () => (props: {children: mixed}) => 3, {
// These will not be hoisted
jest.unmock('../__test_modules__/a').dontMock('../__test_modules__/b');
// eslint-disable-next-line no-useless-concat
jest.unmock('../__test_modules__/' + 'c');
jest.unmock('../__test_modules__/' + 'a');
jest.dontMock('../__test_modules__/Mocked');
{
const jest = {unmock: () => {}};
// Would error (used before initialization) if hoisted to the top of the scope
jest.unmock('../__test_modules__/a');
}

// This must not throw an error
const myObject = {mock: () => {}};
Expand Down Expand Up @@ -84,14 +98,17 @@ describe('babel-plugin-jest-hoist', () => {

expect(d._isMockFunction).toBe(undefined);
expect(d()).toEqual('unmocked');

expect(e._isMock).toBe(undefined);
expect(e()).toEqual('unmocked');
});

it('hoists mock call with 2 arguments', () => {
const path = require('path');

expect(e._isMock).toBe(true);
expect(f._isMock).toBe(true);

const mockFn = e.fn();
const mockFn = f.fn();
expect(mockFn()).toEqual([path.sep, undefined, undefined]);
});

Expand All @@ -100,10 +117,10 @@ describe('babel-plugin-jest-hoist', () => {

global.CALLS = 0;

require('../__test_modules__/e');
require('../__test_modules__/f');
expect(global.CALLS).toEqual(1);

require('../__test_modules__/e');
require('../__test_modules__/f');
expect(global.CALLS).toEqual(1);

delete global.CALLS;
Expand Down
3 changes: 2 additions & 1 deletion packages/babel-plugin-jest-hoist/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
}
},
"dependencies": {
"@babel/template": "^7.3.3",
jeysal marked this conversation as resolved.
Show resolved Hide resolved
"@babel/types": "^7.3.3",
"@types/babel__traverse": "^7.0.6"
},
"devDependencies": {
"@babel/types": "^7.3.3",
"@types/node": "*"
},
"publishConfig": {
Expand Down
197 changes: 156 additions & 41 deletions packages/babel-plugin-jest-hoist/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,21 @@
*
*/

import type {NodePath, Visitor} from '@babel/traverse';
import type {Identifier} from '@babel/types';
import type {NodePath} from '@babel/traverse';
import {
Expression,
Identifier,
Node,
Program,
callExpression,
isIdentifier,
} from '@babel/types';
import {statement} from '@babel/template';
import type {PluginObj} from '@babel/core';

const JEST_GLOBAL_NAME = 'jest';
const JEST_GLOBALS_MODULE_NAME = '@jest/globals';
const JEST_GLOBALS_MODULE_JEST_EXPORT_NAME = 'jest';
SimenB marked this conversation as resolved.
Show resolved Hide resolved

// We allow `jest`, `expect`, `require`, all default Node.js globals and all
// ES2015 built-ins to be used inside of a `jest.mock` factory.
Expand Down Expand Up @@ -70,19 +83,19 @@ const WHITELISTED_IDENTIFIERS = new Set<string>(
].sort(),
);

const JEST_GLOBAL = {name: 'jest'};
// TODO: Should be Visitor<{ids: Set<NodePath<Identifier>>}>, but `ReferencedIdentifier` doesn't exist
const IDVisitor = {
ReferencedIdentifier(path: NodePath<Identifier>) {
// @ts-ignore: passed as Visitor State
this.ids.add(path);
ReferencedIdentifier(
path: NodePath<Identifier>,
{ids}: {ids: Set<NodePath<Identifier>>},
) {
ids.add(path);
},
blacklist: ['TypeAnnotation', 'TSTypeAnnotation', 'TSTypeReference'],
};

const FUNCTIONS: Record<
string,
(args: Array<NodePath>) => boolean
<T extends Node>(args: Array<NodePath<T>>) => boolean
> = Object.create(null);

FUNCTIONS.mock = args => {
Expand All @@ -100,7 +113,7 @@ FUNCTIONS.mock = args => {

const ids: Set<NodePath<Identifier>> = new Set();
const parentScope = moduleFactory.parentPath.scope;
// @ts-ignore: Same as above: ReferencedIdentifier doesn't exist
// @ts-ignore: ReferencedIdentifier is not known on visitors
moduleFactory.traverse(IDVisitor, {ids});
for (const id of ids) {
const {name} = id.node;
Expand Down Expand Up @@ -152,38 +165,140 @@ FUNCTIONS.deepUnmock = args => args.length === 1 && args[0].isStringLiteral();
FUNCTIONS.disableAutomock = FUNCTIONS.enableAutomock = args =>
args.length === 0;

export default (): {visitor: Visitor} => {
const shouldHoistExpression = (expr: NodePath): boolean => {
if (!expr.isCallExpression()) {
return false;
}
const createJestObjectGetter = statement`
function GETTER_NAME() {
const { JEST_GLOBALS_MODULE_JEST_EXPORT_NAME } = require("JEST_GLOBALS_MODULE_NAME");
GETTER_NAME = () => JEST_GLOBALS_MODULE_JEST_EXPORT_NAME;
return JEST_GLOBALS_MODULE_JEST_EXPORT_NAME;
jeysal marked this conversation as resolved.
Show resolved Hide resolved
}
`;

// TODO: avoid type casts - the types can be arrays (is it possible to ignore that without casting?)
const callee = expr.get('callee') as NodePath;
const expressionArguments = expr.get('arguments');
const object = callee.get('object') as NodePath;
const property = callee.get('property') as NodePath;
return (
property.isIdentifier() &&
FUNCTIONS[property.node.name] &&
(object.isIdentifier(JEST_GLOBAL) ||
(callee.isMemberExpression() && shouldHoistExpression(object))) &&
FUNCTIONS[property.node.name](
Array.isArray(expressionArguments)
? expressionArguments
: [expressionArguments],
)
);
};

const visitor: Visitor = {
ExpressionStatement(path) {
if (shouldHoistExpression(path.get('expression') as NodePath)) {
// @ts-ignore: private, magical property
path.node._blockHoist = Infinity;
}
},
};
const isJestObject = (expression: NodePath<Expression>): boolean => {
// global
jeysal marked this conversation as resolved.
Show resolved Hide resolved
if (
expression.isIdentifier() &&
expression.node.name === JEST_GLOBAL_NAME &&
!expression.scope.hasBinding(JEST_GLOBAL_NAME)
) {
return true;
}
// import { jest } from '@jest/globals'
if (
expression.referencesImport(
JEST_GLOBALS_MODULE_NAME,
JEST_GLOBALS_MODULE_JEST_EXPORT_NAME,
)
) {
return true;
}
// import * as JestGlobals from '@jest/globals'
if (
jeysal marked this conversation as resolved.
Show resolved Hide resolved
expression.isMemberExpression() &&
!expression.node.computed &&
jeysal marked this conversation as resolved.
Show resolved Hide resolved
expression
.get<'object'>('object')
.referencesImport(JEST_GLOBALS_MODULE_NAME, '*') &&
expression.node.property.name === JEST_GLOBALS_MODULE_JEST_EXPORT_NAME
) {
return true;
}

return {visitor};
return false;
};

const extractJestObjExprIfHoistable = <T extends Node>(
expr: NodePath<T>,
): NodePath<Expression> | null => {
if (!expr.isCallExpression()) {
return null;
}

const callee = expr.get<'callee'>('callee');
const args = expr.get<'arguments'>('arguments');

if (!callee.isMemberExpression() || callee.node.computed) {
return null;
}

const object = callee.get<'object'>('object');
const property = callee.get<'property'>('property') as NodePath<Identifier>;
const propertyName = property.node.name;

const jestObjExpr = isJestObject(object)
? object
: // The Jest object could be returned from another call since the functions are all chainable.
SimenB marked this conversation as resolved.
Show resolved Hide resolved
extractJestObjExprIfHoistable(object);
if (!jestObjExpr) {
return null;
}

// Important: Call the function check last
// It might throw an error to display to the user,
// which should only happen if we're already sure it's a call on the Jest object.
const functionLooksHoistable = FUNCTIONS[propertyName]?.(args);

return functionLooksHoistable ? jestObjExpr : null;
};

/* eslint-disable sort-keys,@typescript-eslint/explicit-module-boundary-types */
export default (): PluginObj<{
declareJestObjGetterIdentifier: () => Identifier;
jestObjGetterIdentifier?: Identifier;
}> => ({
pre({path: program}: {path: NodePath<Program>}) {
this.declareJestObjGetterIdentifier = () => {
if (this.jestObjGetterIdentifier) {
return this.jestObjGetterIdentifier;
}

this.jestObjGetterIdentifier = program.scope.generateUidIdentifier(
'getJestObj',
);

program.unshiftContainer('body', [
createJestObjectGetter({
GETTER_NAME: this.jestObjGetterIdentifier.name,
JEST_GLOBALS_MODULE_JEST_EXPORT_NAME,
JEST_GLOBALS_MODULE_NAME,
}),
]);

return this.jestObjGetterIdentifier;
};
},
visitor: {
ExpressionStatement(exprStmt) {
const jestObjExpr = extractJestObjExprIfHoistable(
exprStmt.get<'expression'>('expression'),
);
if (jestObjExpr) {
jestObjExpr.replaceWith(
callExpression(this.declareJestObjGetterIdentifier(), []),
);
}
},
},
// in `post` to make sure we come after an import transform and can unshift above the `require`s
post({path: program}: {path: NodePath<Program>}) {
program.traverse({
CallExpression: callExpr => {
const {
node: {callee},
} = callExpr;
if (
isIdentifier(callee) &&
callee.name === this.jestObjGetterIdentifier?.name
) {
const mockStmt = callExpr.getStatementParent();
const mockStmtNode = mockStmt.node;
const mockStmtParent = mockStmt.parentPath;
if (mockStmtParent.isBlock()) {
mockStmt.remove();
mockStmtParent.unshiftContainer('body', [mockStmtNode]);
}
}
},
});
},
});
/* eslint-enable sort-keys,@typescript-eslint/explicit-module-boundary-types */
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@
dependencies:
regenerator-runtime "^0.13.4"

"@babel/template@^7.0.0", "@babel/template@^7.7.4", "@babel/template@^7.8.3", "@babel/template@^7.8.6":
"@babel/template@^7.0.0", "@babel/template@^7.3.3", "@babel/template@^7.7.4", "@babel/template@^7.8.3", "@babel/template@^7.8.6":
version "7.8.6"
resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.8.6.tgz#86b22af15f828dfb086474f964dcc3e39c43ce2b"
integrity sha512-zbMsPMy/v0PWFZEhQJ66bqjhH+z0JgMoBWuikXybgG3Gkd/3t5oQ1Rw2WQhnSrsOmsKXnZOx15tkC4qON/+JPg==
Expand Down