From 6f5009b831f565b0e8992e9a815ba0ff4675a0e8 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Tue, 28 Apr 2020 13:59:44 +0200 Subject: [PATCH] fix: hoist imports of `@jest/globals` correctly (#9806) --- e2e/__tests__/babelPluginJestHoist.test.ts | 4 +- .../__tests__/importJest.test.js | 53 +++++ .../__tests__/integration.test.js | 31 ++- packages/babel-plugin-jest-hoist/package.json | 3 +- packages/babel-plugin-jest-hoist/src/index.ts | 197 ++++++++++++++---- yarn.lock | 2 +- 6 files changed, 238 insertions(+), 52 deletions(-) create mode 100644 e2e/babel-plugin-jest-hoist/__tests__/importJest.test.js diff --git a/e2e/__tests__/babelPluginJestHoist.test.ts b/e2e/__tests__/babelPluginJestHoist.test.ts index 2f7cbd6aafdc..04a2f3e2c9e2 100644 --- a/e2e/__tests__/babelPluginJestHoist.test.ts +++ b/e2e/__tests__/babelPluginJestHoist.test.ts @@ -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); }); diff --git a/e2e/babel-plugin-jest-hoist/__tests__/importJest.test.js b/e2e/babel-plugin-jest-hoist/__tests__/importJest.test.js new file mode 100644 index 000000000000..2933ad991e6e --- /dev/null +++ b/e2e/babel-plugin-jest-hoist/__tests__/importJest.test.js @@ -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'; +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); +}); diff --git a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js index 615f0e31b4e9..3e3dd10de3e2 100644 --- a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js +++ b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js @@ -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'; import jestBackticks from '../__test_modules__/jestBackticks'; // The virtual mock call below will be hoisted above this `require` call. @@ -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; } @@ -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: () => {}}; @@ -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]); }); @@ -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; diff --git a/packages/babel-plugin-jest-hoist/package.json b/packages/babel-plugin-jest-hoist/package.json index de303f8cfdff..60c3d3757bf2 100644 --- a/packages/babel-plugin-jest-hoist/package.json +++ b/packages/babel-plugin-jest-hoist/package.json @@ -20,10 +20,11 @@ } }, "dependencies": { + "@babel/template": "^7.3.3", + "@babel/types": "^7.3.3", "@types/babel__traverse": "^7.0.6" }, "devDependencies": { - "@babel/types": "^7.3.3", "@types/node": "*" }, "publishConfig": { diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 43f19e39c52f..714154256109 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -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'; // We allow `jest`, `expect`, `require`, all default Node.js globals and all // ES2015 built-ins to be used inside of a `jest.mock` factory. @@ -70,19 +83,19 @@ const WHITELISTED_IDENTIFIERS = new Set( ].sort(), ); -const JEST_GLOBAL = {name: 'jest'}; -// TODO: Should be Visitor<{ids: Set>}>, but `ReferencedIdentifier` doesn't exist const IDVisitor = { - ReferencedIdentifier(path: NodePath) { - // @ts-ignore: passed as Visitor State - this.ids.add(path); + ReferencedIdentifier( + path: NodePath, + {ids}: {ids: Set>}, + ) { + ids.add(path); }, blacklist: ['TypeAnnotation', 'TSTypeAnnotation', 'TSTypeReference'], }; const FUNCTIONS: Record< string, - (args: Array) => boolean + (args: Array>) => boolean > = Object.create(null); FUNCTIONS.mock = args => { @@ -100,7 +113,7 @@ FUNCTIONS.mock = args => { const ids: Set> = 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; @@ -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; +} +`; - // 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): boolean => { + // global + 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 ( + expression.isMemberExpression() && + !expression.node.computed && + 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 = ( + expr: NodePath, +): NodePath | 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; + 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. + 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}) { + 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.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 */ diff --git a/yarn.lock b/yarn.lock index 3f604de1b915..9ca11d8bef55 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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==