From b3d939cdcc9368f01d65cfa5a0a96442923b3b85 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 29 Oct 2020 09:24:47 +0100 Subject: [PATCH 1/9] chore: add failing test --- packages/babel-plugin-jest-hoist/package.json | 5 +++- .../src/__tests__/hoistPlugin.test.ts | 30 +++++++++++++++++++ yarn.lock | 5 +++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/package.json b/packages/babel-plugin-jest-hoist/package.json index 5da83c3e267f..4b0496692f2d 100644 --- a/packages/babel-plugin-jest-hoist/package.json +++ b/packages/babel-plugin-jest-hoist/package.json @@ -20,9 +20,12 @@ }, "devDependencies": { "@babel/core": "^7.11.6", + "@babel/preset-react": "^7.12.1", "@types/babel__template": "^7.0.2", "@types/node": "*", - "babel-plugin-tester": "^10.0.0" + "@types/prettier": "^2.0.0", + "babel-plugin-tester": "^10.0.0", + "prettier": "^2.1.1" }, "publishConfig": { "access": "public" diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts index 1e0554a2958c..f7057f859dad 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts +++ b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts @@ -6,13 +6,43 @@ * */ +import * as path from 'path'; import pluginTester from 'babel-plugin-tester'; +import {format as formatCode} from 'prettier'; import babelPluginJestHoist from '..'; +const fakeAbsolutPath = path.resolve(__dirname, '../file.js'); + pluginTester({ plugin: babelPluginJestHoist, pluginName: 'babel-plugin-jest-hoist', tests: { + 'automatic react runtime': { + babelOptions: { + babelrc: false, + configFile: false, + filename: fakeAbsolutPath, + presets: [ + [ + require.resolve('@babel/preset-react'), + {development: true, runtime: 'automatic'}, + ], + ], + }, + code: ` + jest.mock('./App', () => () =>
Hello world
); + `, + formatResult(code) { + // replace the filename with something that will be the same across OSes and machine + const codeWithoutSystemPath = code.replace( + new RegExp(fakeAbsolutPath, 'g'), + '/root/project/src/file.js', + ); + + return formatCode(codeWithoutSystemPath, {parser: 'babel'}); + }, + snapshot: true, + }, 'top level mocking': { code: ` require('x'); diff --git a/yarn.lock b/yarn.lock index 6f5f7eece467..bbd5d5e0e225 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1456,7 +1456,7 @@ __metadata: languageName: node linkType: hard -"@babel/preset-react@npm:*, @babel/preset-react@npm:^7.0.0, @babel/preset-react@npm:^7.9.4": +"@babel/preset-react@npm:*, @babel/preset-react@npm:^7.0.0, @babel/preset-react@npm:^7.12.1, @babel/preset-react@npm:^7.9.4": version: 7.12.1 resolution: "@babel/preset-react@npm:7.12.1" dependencies: @@ -4830,13 +4830,16 @@ __metadata: resolution: "babel-plugin-jest-hoist@workspace:packages/babel-plugin-jest-hoist" dependencies: "@babel/core": ^7.11.6 + "@babel/preset-react": ^7.12.1 "@babel/template": ^7.3.3 "@babel/types": ^7.3.3 "@types/babel__core": ^7.0.0 "@types/babel__template": ^7.0.2 "@types/babel__traverse": ^7.0.6 "@types/node": "*" + "@types/prettier": ^2.0.0 babel-plugin-tester: ^10.0.0 + prettier: ^2.1.1 languageName: unknown linkType: soft From fdb6bcce8c45d1d0849df1071514b6c3b2cf92ae Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 29 Oct 2020 09:55:50 +0100 Subject: [PATCH 2/9] hack a "fix" --- .../__snapshots__/hoistPlugin.test.ts.snap | 37 +++++++++++++++++++ packages/babel-plugin-jest-hoist/src/index.ts | 15 +++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap index 0c1a8424ff4f..bb0deabba40c 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap +++ b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap @@ -1,5 +1,42 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`babel-plugin-jest-hoist automatic react runtime: automatic react runtime 1`] = ` + +jest.mock('./App', () => () =>
Hello world
); + + ↓ ↓ ↓ ↓ ↓ ↓ + +_getJestObj().mock("./App", () => () => + /*#__PURE__*/ _jsxDEV( + "div", + { + children: "Hello world" + }, + void 0, + false, + { + fileName: _jsxFileName, + lineNumber: 1, + columnNumber: 32 + }, + this + ) +); + +import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime"; +var _jsxFileName = "/root/project/src/file.js"; + +function _getJestObj() { + const { jest } = require("@jest/globals"); + + _getJestObj = () => jest; + + return jest; +} + + +`; + exports[`babel-plugin-jest-hoist top level mocking: top level mocking 1`] = ` require('x'); diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 99e0d8b50749..6d02aefefefa 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -133,12 +133,25 @@ FUNCTIONS.mock = args => { } if (!found) { - const isAllowedIdentifier = + let isAllowedIdentifier = (scope.hasGlobal(name) && ALLOWED_IDENTIFIERS.has(name)) || /^mock/i.test(name) || // Allow istanbul's coverage variable to pass. /^(?:__)?cov/.test(name); + if (!isAllowedIdentifier) { + const binding = scope.bindings[name]; + + if (binding?.path.isVariableDeclarator()) { + const initNode = binding.path.node.init; + + if (initNode && binding.constant && scope.isPure(initNode, true)) { + // how to hoist??? + isAllowedIdentifier = true; + } + } + } + if (!isAllowedIdentifier) { throw id.buildCodeFrameError( 'The module factory of `jest.mock()` is not allowed to ' + From aa31e81d8c33212fb2754cd869d7ff224a361d90 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 1 Nov 2020 02:03:29 +0100 Subject: [PATCH 3/9] replace variable reference with constant value --- .../src/__tests__/__snapshots__/hoistPlugin.test.ts.snap | 2 +- packages/babel-plugin-jest-hoist/src/index.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap index bb0deabba40c..227860db32e3 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap +++ b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap @@ -15,7 +15,7 @@ _getJestObj().mock("./App", () => () => void 0, false, { - fileName: _jsxFileName, + fileName: "/root/project/src/file.js", lineNumber: 1, columnNumber: 32 }, diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 6d02aefefefa..f952746674e6 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -146,7 +146,9 @@ FUNCTIONS.mock = args => { const initNode = binding.path.node.init; if (initNode && binding.constant && scope.isPure(initNode, true)) { - // how to hoist??? + // replace the reference with its constant value + id.replaceWith(initNode); + isAllowedIdentifier = true; } } @@ -286,7 +288,7 @@ export default (): PluginObj<{ visitor: { ExpressionStatement(exprStmt) { const jestObjExpr = extractJestObjExprIfHoistable( - exprStmt.get<'expression'>('expression'), + exprStmt.get('expression'), ); if (jestObjExpr) { jestObjExpr.replaceWith( From 045af2b548644feb6c0c1ad043ec2c91dab16ccd Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 1 Nov 2020 02:45:18 +0100 Subject: [PATCH 4/9] use a function --- .../__snapshots__/hoistPlugin.test.ts.snap | 6 +++++- packages/babel-plugin-jest-hoist/src/index.ts | 21 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap index 227860db32e3..3255331311b5 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap +++ b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap @@ -15,7 +15,7 @@ _getJestObj().mock("./App", () => () => void 0, false, { - fileName: "/root/project/src/file.js", + fileName: _hoistedMockFactoryVariable(), lineNumber: 1, columnNumber: 32 }, @@ -34,6 +34,10 @@ function _getJestObj() { return jest; } +function _hoistedMockFactoryVariable() { + return "/root/project/src/file.js"; +} + `; diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index f952746674e6..7ecafb8847da 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -146,8 +146,19 @@ FUNCTIONS.mock = args => { const initNode = binding.path.node.init; if (initNode && binding.constant && scope.isPure(initNode, true)) { - // replace the reference with its constant value - id.replaceWith(initNode); + const identifier = scope.generateUidIdentifier( + 'hoistedMockFactoryVariable', + ); + + binding.path.parentPath.insertAfter( + createHoistedVariable({ + HOISTED_NAME: identifier, + HOISTED_VALUE: binding.path.node.init, + }), + ); + + // replace reference with a call to the new function + id.replaceWith(callExpression(identifier, [])); isAllowedIdentifier = true; } @@ -191,6 +202,12 @@ function GETTER_NAME() { } `; +const createHoistedVariable = statement` +function HOISTED_NAME() { + return HOISTED_VALUE; +} +`; + const isJestObject = (expression: NodePath): boolean => { // global if ( From f294765603d6ae4774d54acb658b9131abfdcef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sun, 1 Nov 2020 23:32:42 +0100 Subject: [PATCH 5/9] Hoist static variables before hoisted mocks (#653) --- .../__snapshots__/hoistPlugin.test.ts.snap | 9 ++-- packages/babel-plugin-jest-hoist/src/index.ts | 53 ++++++++++--------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap index 3255331311b5..09486e91ae92 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap +++ b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap @@ -6,6 +6,8 @@ jest.mock('./App', () => () =>
Hello world
); ↓ ↓ ↓ ↓ ↓ ↓ +var _jsxFileName = "/root/project/src/file.js"; + _getJestObj().mock("./App", () => () => /*#__PURE__*/ _jsxDEV( "div", @@ -15,7 +17,7 @@ _getJestObj().mock("./App", () => () => void 0, false, { - fileName: _hoistedMockFactoryVariable(), + fileName: _jsxFileName, lineNumber: 1, columnNumber: 32 }, @@ -24,7 +26,6 @@ _getJestObj().mock("./App", () => () => ); import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime"; -var _jsxFileName = "/root/project/src/file.js"; function _getJestObj() { const { jest } = require("@jest/globals"); @@ -34,10 +35,6 @@ function _getJestObj() { return jest; } -function _hoistedMockFactoryVariable() { - return "/root/project/src/file.js"; -} - `; diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 7ecafb8847da..e6d37508871f 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -16,15 +16,20 @@ import { Identifier, Node, Program, + VariableDeclaration, + VariableDeclarator, callExpression, emptyStatement, isIdentifier, + variableDeclaration, } from '@babel/types'; const JEST_GLOBAL_NAME = 'jest'; const JEST_GLOBALS_MODULE_NAME = '@jest/globals'; const JEST_GLOBALS_MODULE_JEST_EXPORT_NAME = 'jest'; +const hoistedVariables = new WeakSet(); + // We allow `jest`, `expect`, `require`, all default Node.js globals and all // ES2015 built-ins to be used inside of a `jest.mock` factory. // We also allow variables prefixed with `mock` as an escape-hatch. @@ -146,20 +151,7 @@ FUNCTIONS.mock = args => { const initNode = binding.path.node.init; if (initNode && binding.constant && scope.isPure(initNode, true)) { - const identifier = scope.generateUidIdentifier( - 'hoistedMockFactoryVariable', - ); - - binding.path.parentPath.insertAfter( - createHoistedVariable({ - HOISTED_NAME: identifier, - HOISTED_VALUE: binding.path.node.init, - }), - ); - - // replace reference with a call to the new function - id.replaceWith(callExpression(identifier, [])); - + hoistedVariables.add(binding.path.node); isAllowedIdentifier = true; } } @@ -202,12 +194,6 @@ function GETTER_NAME() { } `; -const createHoistedVariable = statement` -function HOISTED_NAME() { - return HOISTED_VALUE; -} -`; - const isJestObject = (expression: NodePath): boolean => { // global if ( @@ -317,6 +303,7 @@ export default (): PluginObj<{ // in `post` to make sure we come after an import transform and can unshift above the `require`s post({path: program}) { const self = this; + visitBlock(program); program.traverse({ BlockStatement: visitBlock, @@ -324,17 +311,19 @@ export default (): PluginObj<{ function visitBlock(block: NodePath | NodePath) { // use a temporary empty statement instead of the real first statement, which may itself be hoisted - const [firstNonHoistedStatementOfBlock] = block.unshiftContainer( - 'body', + const [varsHoistPoint, callsHoistPoint] = block.unshiftContainer('body', [ emptyStatement(), - ); + emptyStatement(), + ]); block.traverse({ CallExpression: visitCallExpr, + VariableDeclarator: visitVariableDeclarator, // do not traverse into nested blocks, or we'll hoist calls in there out to this block // @ts-expect-error blacklist is not known blacklist: ['BlockStatement'], }); - firstNonHoistedStatementOfBlock.remove(); + callsHoistPoint.remove(); + varsHoistPoint.remove(); function visitCallExpr(callExpr: NodePath) { const { @@ -351,11 +340,25 @@ export default (): PluginObj<{ const mockStmtParent = mockStmt.parentPath; if (mockStmtParent.isBlock()) { mockStmt.remove(); - firstNonHoistedStatementOfBlock.insertBefore(mockStmtNode); + callsHoistPoint.insertBefore(mockStmtNode); } } } } + + function visitVariableDeclarator(varDecl: NodePath) { + if (hoistedVariables.has(varDecl.node)) { + const {kind, declarations} = varDecl.parent as VariableDeclaration; + if (declarations.length === 1) { + varDecl.parentPath.remove(); + } else { + varDecl.remove(); + } + varsHoistPoint.insertBefore( + variableDeclaration(kind, [varDecl.node]), + ); + } + } } }, }); From 8efd7dfd7f032c54a159b55d171076e6e59abe6f Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 1 Nov 2020 23:37:53 +0100 Subject: [PATCH 6/9] maybe fix test --- .../src/__tests__/hoistPlugin.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts index f7057f859dad..65761852705e 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts +++ b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts @@ -11,8 +11,6 @@ import pluginTester from 'babel-plugin-tester'; import {format as formatCode} from 'prettier'; import babelPluginJestHoist from '..'; -const fakeAbsolutPath = path.resolve(__dirname, '../file.js'); - pluginTester({ plugin: babelPluginJestHoist, pluginName: 'babel-plugin-jest-hoist', @@ -21,7 +19,7 @@ pluginTester({ babelOptions: { babelrc: false, configFile: false, - filename: fakeAbsolutPath, + filename: path.resolve(__dirname, '../file.js'), presets: [ [ require.resolve('@babel/preset-react'), @@ -35,8 +33,8 @@ pluginTester({ formatResult(code) { // replace the filename with something that will be the same across OSes and machine const codeWithoutSystemPath = code.replace( - new RegExp(fakeAbsolutPath, 'g'), - '/root/project/src/file.js', + /var _jsxFileName = ".*";/, + 'var _jsxFileName = "/root/project/src/file.js";', ); return formatCode(codeWithoutSystemPath, {parser: 'babel'}); From 4e03be614f6d3d983b39dc400e2e0eafb4f2d9ae Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 2 Nov 2020 00:01:29 +0100 Subject: [PATCH 7/9] add assert --- packages/babel-plugin-jest-hoist/src/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index e6d37508871f..f5c59c44de25 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -148,10 +148,11 @@ FUNCTIONS.mock = args => { const binding = scope.bindings[name]; if (binding?.path.isVariableDeclarator()) { - const initNode = binding.path.node.init; + const {node} = binding.path; + const initNode = node.init; if (initNode && binding.constant && scope.isPure(initNode, true)) { - hoistedVariables.add(binding.path.node); + hoistedVariables.add(node); isAllowedIdentifier = true; } } @@ -305,9 +306,7 @@ export default (): PluginObj<{ const self = this; visitBlock(program); - program.traverse({ - BlockStatement: visitBlock, - }); + program.traverse({BlockStatement: visitBlock}); function visitBlock(block: NodePath | NodePath) { // use a temporary empty statement instead of the real first statement, which may itself be hoisted @@ -336,9 +335,9 @@ export default (): PluginObj<{ const mockStmt = callExpr.getStatementParent(); if (mockStmt) { - const mockStmtNode = mockStmt.node; const mockStmtParent = mockStmt.parentPath; if (mockStmtParent.isBlock()) { + const mockStmtNode = mockStmt.node; mockStmt.remove(); callsHoistPoint.insertBefore(mockStmtNode); } @@ -348,6 +347,9 @@ export default (): PluginObj<{ function visitVariableDeclarator(varDecl: NodePath) { if (hoistedVariables.has(varDecl.node)) { + // should be assert function, but it's not. So let's cast below + varDecl.parentPath.assertVariableDeclaration(); + const {kind, declarations} = varDecl.parent as VariableDeclaration; if (declarations.length === 1) { varDecl.parentPath.remove(); From 27a136203fbcbc3bea31559cb75e9cbf85fb3773 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 2 Nov 2020 00:02:42 +0100 Subject: [PATCH 8/9] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0017592475c4..9fa6cd4532f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes - `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10536](https://github.com/facebook/jest/pull/10536)) +- `[babel-plugin-jest-hoist]` Hoist pure constants to support experimental JSX transform in hoisted mocks - `[babel-preset-jest]` Update `babel-preset-current-node-syntax` to support top level await ([#10747](https://github.com/facebook/jest/pull/10747)) - `[expect]` Stop modifying the sample in `expect.objectContaining()` ([#10711](https://github.com/facebook/jest/pull/10711)) - `[jest-circus, jest-jasmine2]` fix: don't assume `stack` is always a string ([#10697](https://github.com/facebook/jest/pull/10697)) From 70a68776551a06e939354fce6750c10c747cb13d Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 2 Nov 2020 00:04:12 +0100 Subject: [PATCH 9/9] link in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa6cd4532f3..c652a32acd75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixes - `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10536](https://github.com/facebook/jest/pull/10536)) -- `[babel-plugin-jest-hoist]` Hoist pure constants to support experimental JSX transform in hoisted mocks +- `[babel-plugin-jest-hoist]` Hoist pure constants to support experimental JSX transform in hoisted mocks ([#10723](https://github.com/facebook/jest/pull/10723)) - `[babel-preset-jest]` Update `babel-preset-current-node-syntax` to support top level await ([#10747](https://github.com/facebook/jest/pull/10747)) - `[expect]` Stop modifying the sample in `expect.objectContaining()` ([#10711](https://github.com/facebook/jest/pull/10711)) - `[jest-circus, jest-jasmine2]` fix: don't assume `stack` is always a string ([#10697](https://github.com/facebook/jest/pull/10697))