From f7fe79573500c1b8974b84c3f29a419b2c24e419 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sat, 19 Sep 2020 13:27:05 -0400 Subject: [PATCH 01/12] failing test for hoist order --- .../__tests__/hoistOrder.test.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js diff --git a/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js b/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js new file mode 100644 index 000000000000..5fe684947e1d --- /dev/null +++ b/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js @@ -0,0 +1,21 @@ +/** + * 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. + * + */ + +'use strict'; + +import a from '../__test_modules__/a'; + +// These will all be hoisted above imports +jest.enableAutomock(); +jest.disableAutomock(); + +describe('babel-plugin-jest-hoist', () => { + it('preserves hoist order of disableAutomock & enableAutomock', () => { + expect(a._isMockFunction).toBe(undefined); + }); +}); From fe4b5b5f102ff8a520b013a008f568b113b471be Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sat, 19 Sep 2020 13:28:37 -0400 Subject: [PATCH 02/12] fix: Unshift mock statements in the order they are defined --- packages/babel-plugin-jest-hoist/src/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 8fbab87dc0f4..7ea3e47f60cb 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -281,6 +281,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}: {path: NodePath}) { + const mockStmts: Array = []; program.traverse({ CallExpression: callExpr => { const { @@ -297,12 +298,13 @@ export default (): PluginObj<{ const mockStmtParent = mockStmt.parentPath; if (mockStmtParent.isBlock()) { mockStmt.remove(); - mockStmtParent.unshiftContainer('body', [mockStmtNode]); + mockStmts.push(mockStmtNode); } } } }, }); + program.unshiftContainer('body', mockStmts); }, }); /* eslint-enable */ From ad0b30f9584cb1511a93d5b944ba2e49d5ad1c82 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 11:55:06 -0400 Subject: [PATCH 03/12] Fix hoist implementation, update CHANGELOG.md --- CHANGELOG.md | 1 + packages/babel-plugin-jest-hoist/src/index.ts | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a0b62f24f55..0c5568e291cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ ### Fixes +- `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10413](https://github.com/facebook/jest/pull/10413)) - `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) - `[jest-console]` Add `Console` constructor to `console` object ([#10502](https://github.com/facebook/jest/pull/10502)) - `[jest-globals]` Fix lifecycle hook function types ([#10480](https://github.com/facebook/jest/pull/10480)) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 7ea3e47f60cb..92ebaab6bb08 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -21,6 +21,7 @@ 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'; +const JEST_HOIST_COUNT = '__babelJestHoistNodesCount'; // We allow `jest`, `expect`, `require`, all default Node.js globals and all // ES2015 built-ins to be used inside of a `jest.mock` factory. @@ -281,7 +282,6 @@ export default (): PluginObj<{ }, // in `post` to make sure we come after an import transform and can unshift above the `require`s post({path: program}: {path: NodePath}) { - const mockStmts: Array = []; program.traverse({ CallExpression: callExpr => { const { @@ -298,13 +298,30 @@ export default (): PluginObj<{ const mockStmtParent = mockStmt.parentPath; if (mockStmtParent.isBlock()) { mockStmt.remove(); - mockStmts.push(mockStmtNode); + const hoistNodesCount = + mockStmtParent.getData(JEST_HOIST_COUNT) ?? null; + // Ensures we preserve order while hoisting, only unshift the + // first node, otherwise insert after the previously hoisted node + if (hoistNodesCount !== null) { + const lastHoisted = mockStmtParent.get( + `body.${hoistNodesCount - 1}`, + ); + if (Array.isArray(lastHoisted)) { + throw new Error('Invariant'); + } + lastHoisted.insertAfter(mockStmtNode); + } else { + mockStmtParent.unshiftContainer('body', [mockStmtNode]); + } + mockStmtParent.setData( + JEST_HOIST_COUNT, + (hoistNodesCount ?? 0) + 1, + ); } } } }, }); - program.unshiftContainer('body', mockStmts); }, }); /* eslint-enable */ From b987dab1209e8be633a5be52573023c828ff9a76 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 11:57:30 -0400 Subject: [PATCH 04/12] Fix changelog issue number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5568e291cf..b9869e1d64cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,7 +91,7 @@ ### Fixes -- `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10413](https://github.com/facebook/jest/pull/10413)) +- `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10536](https://github.com/facebook/jest/pull/10536)) - `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) - `[jest-console]` Add `Console` constructor to `console` object ([#10502](https://github.com/facebook/jest/pull/10502)) - `[jest-globals]` Fix lifecycle hook function types ([#10480](https://github.com/facebook/jest/pull/10480)) From b92e6d89146519738fc395c877aae9917f5ead91 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 16:24:02 -0400 Subject: [PATCH 05/12] Fix babel plugin, add snapshot tests --- packages/babel-plugin-jest-hoist/package.json | 3 +- .../__snapshots__/hoistPlugin.test.ts.snap | 76 +++++++++++++++++++ .../src/__tests__/hoistPlugin.test.ts | 35 +++++++++ packages/babel-plugin-jest-hoist/src/index.ts | 50 ++++-------- yarn.lock | 43 ++++++++++- 5 files changed, 167 insertions(+), 40 deletions(-) create mode 100644 packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap create mode 100644 packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts diff --git a/packages/babel-plugin-jest-hoist/package.json b/packages/babel-plugin-jest-hoist/package.json index 8670cb905ecd..877cdc17cb8c 100644 --- a/packages/babel-plugin-jest-hoist/package.json +++ b/packages/babel-plugin-jest-hoist/package.json @@ -20,7 +20,8 @@ }, "devDependencies": { "@types/babel__template": "^7.0.2", - "@types/node": "*" + "@types/node": "*", + "babel-plugin-tester": "9.2.0" }, "publishConfig": { "access": "public" 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 new file mode 100644 index 000000000000..0c1a8424ff4f --- /dev/null +++ b/packages/babel-plugin-jest-hoist/src/__tests__/__snapshots__/hoistPlugin.test.ts.snap @@ -0,0 +1,76 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`babel-plugin-jest-hoist top level mocking: top level mocking 1`] = ` + +require('x'); + +jest.enableAutomock(); +jest.disableAutomock(); + + ↓ ↓ ↓ ↓ ↓ ↓ + +_getJestObj().enableAutomock(); + +_getJestObj().disableAutomock(); + +function _getJestObj() { + const { jest } = require("@jest/globals"); + + _getJestObj = () => jest; + + return jest; +} + +require("x"); + + +`; + +exports[`babel-plugin-jest-hoist within a block with no siblings: within a block with no siblings 1`] = ` + +beforeEach(() => { + jest.mock('someNode') +}) + + ↓ ↓ ↓ ↓ ↓ ↓ + +function _getJestObj() { + const { jest } = require("@jest/globals"); + + _getJestObj = () => jest; + + return jest; +} + +beforeEach(() => { + _getJestObj().mock("someNode"); +}); + + +`; + +exports[`babel-plugin-jest-hoist within a block: within a block 1`] = ` + +beforeEach(() => { + require('x') + jest.mock('someNode') +}) + + ↓ ↓ ↓ ↓ ↓ ↓ + +function _getJestObj() { + const { jest } = require("@jest/globals"); + + _getJestObj = () => jest; + + return jest; +} + +beforeEach(() => { + _getJestObj().mock("someNode"); + + require("x"); +}); + + +`; diff --git a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts new file mode 100644 index 000000000000..5d32df5d143e --- /dev/null +++ b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts @@ -0,0 +1,35 @@ +import pluginTester from 'babel-plugin-tester'; +import babelPluginJestHoist from '..'; + +pluginTester({ + plugin: babelPluginJestHoist, + pluginName: 'babel-plugin-jest-hoist', + tests: { + 'top level mocking': { + code: ` + require('x'); + + jest.enableAutomock(); + jest.disableAutomock(); + `, + snapshot: true, + }, + 'within a block': { + code: ` + beforeEach(() => { + require('x') + jest.mock('someNode') + }) + `, + snapshot: true, + }, + 'within a block with no siblings': { + code: ` + beforeEach(() => { + jest.mock('someNode') + }) + `, + snapshot: true, + }, + }, +}); diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 92ebaab6bb08..dc2a7ef7ced9 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -13,7 +13,7 @@ import { Node, Program, callExpression, - isIdentifier, + clone, } from '@babel/types'; import {statement} from '@babel/template'; import type {PluginObj} from '@babel/core'; @@ -21,7 +21,6 @@ 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'; -const JEST_HOIST_COUNT = '__babelJestHoistNodesCount'; // We allow `jest`, `expect`, `require`, all default Node.js globals and all // ES2015 built-ins to be used inside of a `jest.mock` factory. @@ -277,47 +276,24 @@ export default (): PluginObj<{ jestObjExpr.replaceWith( callExpression(this.declareJestObjGetterIdentifier(), []), ); + exprStmt.setData('shouldHoist', true); } }, }, // 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(); - - if (mockStmt) { - const mockStmtNode = mockStmt.node; - const mockStmtParent = mockStmt.parentPath; - if (mockStmtParent.isBlock()) { - mockStmt.remove(); - const hoistNodesCount = - mockStmtParent.getData(JEST_HOIST_COUNT) ?? null; - // Ensures we preserve order while hoisting, only unshift the - // first node, otherwise insert after the previously hoisted node - if (hoistNodesCount !== null) { - const lastHoisted = mockStmtParent.get( - `body.${hoistNodesCount - 1}`, - ); - if (Array.isArray(lastHoisted)) { - throw new Error('Invariant'); - } - lastHoisted.insertAfter(mockStmtNode); - } else { - mockStmtParent.unshiftContainer('body', [mockStmtNode]); - } - mockStmtParent.setData( - JEST_HOIST_COUNT, - (hoistNodesCount ?? 0) + 1, - ); - } + ExpressionStatement: exprStmt => { + if (exprStmt.getData('shouldHoist')) { + const prevSiblings = exprStmt.getAllPrevSiblings(); + const unhoistedSiblings = prevSiblings + .reverse() + .filter(s => !s.getData('shouldHoist') && !s.getData('wasHoisted')); + const exprNode = exprStmt.node; + if (unhoistedSiblings.length) { + const hoisted = unhoistedSiblings[0].insertBefore(clone(exprNode)); + hoisted[0].setData('wasHoisted', true); + exprStmt.remove(); } } }, diff --git a/yarn.lock b/yarn.lock index 88e1380fe069..1a2434ba4635 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3251,6 +3251,16 @@ __metadata: languageName: node linkType: hard +"@types/babel-plugin-tester@npm:^9.0.0": + version: 9.0.0 + resolution: "@types/babel-plugin-tester@npm:9.0.0" + dependencies: + "@types/babel__core": "*" + "@types/prettier": "*" + checksum: d6b465ef6ce980927dd3b10ae0a39fd8384688ba2d15bf2a30ac4ae567572abb2768992e7c2389f873c87c8d9ba5eafe88feb399e1e9e93f3cbfc5fe27da7077 + languageName: node + linkType: hard + "@types/babel-types@npm:*": version: 7.0.9 resolution: "@types/babel-types@npm:7.0.9" @@ -3265,7 +3275,7 @@ __metadata: languageName: node linkType: hard -"@types/babel__core@npm:^7.0.0, @types/babel__core@npm:^7.0.4, @types/babel__core@npm:^7.1.0, @types/babel__core@npm:^7.1.7": +"@types/babel__core@npm:*, @types/babel__core@npm:^7.0.0, @types/babel__core@npm:^7.0.4, @types/babel__core@npm:^7.1.0, @types/babel__core@npm:^7.1.7": version: 7.1.7 resolution: "@types/babel__core@npm:7.1.7" dependencies: @@ -3638,6 +3648,13 @@ __metadata: languageName: node linkType: hard +"@types/prettier@npm:*": + version: 2.1.1 + resolution: "@types/prettier@npm:2.1.1" + checksum: 3671bedc845a0e61bb8eb698746e1f6d1201ac784f95c536cd653c1406a51c0e9c338ecbbc73f1b5fd5fe0b0af98edf7e85428810357d959355ab46b3a63ebe6 + languageName: node + linkType: hard + "@types/prettier@npm:^2.0.0": version: 2.1.5 resolution: "@types/prettier@npm:2.1.5" @@ -4813,6 +4830,7 @@ __metadata: "@types/babel__template": ^7.0.2 "@types/babel__traverse": ^7.0.6 "@types/node": "*" + babel-plugin-tester: 9.2.0 languageName: unknown linkType: soft @@ -4830,6 +4848,20 @@ __metadata: languageName: node linkType: hard +"babel-plugin-tester@npm:9.2.0": + version: 9.2.0 + resolution: "babel-plugin-tester@npm:9.2.0" + dependencies: + "@types/babel-plugin-tester": ^9.0.0 + lodash.mergewith: ^4.6.2 + prettier: ^2.0.1 + strip-indent: ^3.0.0 + peerDependencies: + "@babel/core": ^7.9.0 + checksum: ce247d30010fc4e7f28733187a6b347f292226078a72c58c399d2f30f5ec2a054cdfe81831d3455292a58ab3db3df35cae1db8a44234a35466090b90075a619c + languageName: node + linkType: hard + "babel-plugin-transform-typescript-metadata@npm:*": version: 0.3.1 resolution: "babel-plugin-transform-typescript-metadata@npm:0.3.1" @@ -12841,6 +12873,13 @@ fsevents@^1.2.7: languageName: node linkType: hard +"lodash.mergewith@npm:^4.6.2": + version: 4.6.2 + resolution: "lodash.mergewith@npm:4.6.2" + checksum: 3561b63cebc629721ab4c016627fc54929ee33cdef1854b4a15ade71dd8eb5f2fc602830efe5395aed41c607d65e2cce356667116aa7156b82468594b42ab95f + languageName: node + linkType: hard + "lodash.padstart@npm:^4.6.1": version: 4.6.1 resolution: "lodash.padstart@npm:4.6.1" @@ -15865,7 +15904,7 @@ fsevents@^1.2.7: languageName: node linkType: hard -"prettier@npm:^2.1.1": +"prettier@npm:^2.0.1, prettier@npm:^2.1.1": version: 2.1.2 resolution: "prettier@npm:2.1.2" bin: From 619987ba1b6dd50219b09118a6b80ce77b6c0ae1 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 16:40:00 -0400 Subject: [PATCH 06/12] Add Copyright header, better name data keys --- .../src/__tests__/hoistPlugin.test.ts | 8 ++++++++ packages/babel-plugin-jest-hoist/src/index.ts | 11 +++++++---- 2 files changed, 15 insertions(+), 4 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 5d32df5d143e..1e0554a2958c 100644 --- a/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts +++ b/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts @@ -1,3 +1,11 @@ +/** + * 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. + * + */ + import pluginTester from 'babel-plugin-tester'; import babelPluginJestHoist from '..'; diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index dc2a7ef7ced9..5da3dbd0d825 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -276,7 +276,7 @@ export default (): PluginObj<{ jestObjExpr.replaceWith( callExpression(this.declareJestObjGetterIdentifier(), []), ); - exprStmt.setData('shouldHoist', true); + exprStmt.setData('_jestShouldHoist', true); } }, }, @@ -284,15 +284,18 @@ export default (): PluginObj<{ post({path: program}: {path: NodePath}) { program.traverse({ ExpressionStatement: exprStmt => { - if (exprStmt.getData('shouldHoist')) { + if (exprStmt.getData('_jestShouldHoist')) { const prevSiblings = exprStmt.getAllPrevSiblings(); const unhoistedSiblings = prevSiblings .reverse() - .filter(s => !s.getData('shouldHoist') && !s.getData('wasHoisted')); + .filter( + s => + !s.getData('_jestShouldHoist') && !s.getData('_jestWasHoisted'), + ); const exprNode = exprStmt.node; if (unhoistedSiblings.length) { const hoisted = unhoistedSiblings[0].insertBefore(clone(exprNode)); - hoisted[0].setData('wasHoisted', true); + hoisted[0].setData('_jestWasHoisted', true); exprStmt.remove(); } } From 60dc8d218bc5f4bc54bce2ba175240296055a217 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 17:11:28 -0400 Subject: [PATCH 07/12] Fix broken integration test due to proper hoist ordering --- e2e/babel-plugin-jest-hoist/__tests__/integration.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js index 066a60bb89c9..0650c47a5cd3 100644 --- a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js +++ b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js @@ -34,6 +34,10 @@ let e; jest.unmock('../__test_modules__/e'); })(); +// Variable names prefixed with `mock` (ignore case) should not throw as out-of-scope +const MockMethods = () => {}; +jest.mock('../__test_modules__/f', () => MockMethods); + jest.mock('../__test_modules__/f', () => { if (!global.CALLS) { global.CALLS = 0; @@ -73,10 +77,6 @@ jest.dontMock('../__test_modules__/Mocked'); const myObject = {mock: () => {}}; myObject.mock('apple', 27); -// Variable names prefixed with `mock` (ignore case) should not throw as out-of-scope -const MockMethods = () => {}; -jest.mock('../__test_modules__/f', () => MockMethods); - describe('babel-plugin-jest-hoist', () => { it('does not throw during transform', () => { const object = {}; From 19cba40fd0e81f48f8171006c50a76d363f1e667 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 21 Sep 2020 17:33:00 -0400 Subject: [PATCH 08/12] Remove extraneous e2e test --- .../__tests__/hoistOrder.test.js | 21 ------------------- 1 file changed, 21 deletions(-) delete mode 100644 e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js diff --git a/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js b/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js deleted file mode 100644 index 5fe684947e1d..000000000000 --- a/e2e/babel-plugin-jest-hoist/__tests__/hoistOrder.test.js +++ /dev/null @@ -1,21 +0,0 @@ -/** - * 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. - * - */ - -'use strict'; - -import a from '../__test_modules__/a'; - -// These will all be hoisted above imports -jest.enableAutomock(); -jest.disableAutomock(); - -describe('babel-plugin-jest-hoist', () => { - it('preserves hoist order of disableAutomock & enableAutomock', () => { - expect(a._isMockFunction).toBe(undefined); - }); -}); From c23e34816c7b6b45e79625db74f7fbb7f353c823 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 28 Oct 2020 09:32:33 +0100 Subject: [PATCH 09/12] test correction --- e2e/babel-plugin-jest-hoist/__tests__/integration.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js index 0650c47a5cd3..4b534faa9fb2 100644 --- a/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js +++ b/e2e/babel-plugin-jest-hoist/__tests__/integration.test.js @@ -34,10 +34,6 @@ let e; jest.unmock('../__test_modules__/e'); })(); -// Variable names prefixed with `mock` (ignore case) should not throw as out-of-scope -const MockMethods = () => {}; -jest.mock('../__test_modules__/f', () => MockMethods); - jest.mock('../__test_modules__/f', () => { if (!global.CALLS) { global.CALLS = 0; @@ -77,6 +73,10 @@ jest.dontMock('../__test_modules__/Mocked'); const myObject = {mock: () => {}}; myObject.mock('apple', 27); +// Variable names prefixed with `mock` (ignore case) should not throw as out-of-scope +const MockMethods = () => {}; +jest.mock('../__test_modules__/g', () => MockMethods); + describe('babel-plugin-jest-hoist', () => { it('does not throw during transform', () => { const object = {}; From 8bb537209cd046691516d72a4276820428c48051 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 28 Oct 2020 21:09:31 +0100 Subject: [PATCH 10/12] refactor: pure traversal solution --- packages/babel-plugin-jest-hoist/src/index.ts | 62 +++++++++++++------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 5da3dbd0d825..15e2c5bf5ae7 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -8,12 +8,15 @@ import type {NodePath} from '@babel/traverse'; import { + BlockStatement, + CallExpression, Expression, Identifier, Node, Program, callExpression, - clone, + emptyStatement, + isIdentifier, } from '@babel/types'; import {statement} from '@babel/template'; import type {PluginObj} from '@babel/core'; @@ -113,7 +116,7 @@ FUNCTIONS.mock = args => { const ids: Set> = new Set(); const parentScope = moduleFactory.parentPath.scope; - // @ts-expect-error: ReferencedIdentifier is not known on visitors + // @ts-expect-error: ReferencedIdentifier and blacklist are not known on visitors moduleFactory.traverse(IDVisitor, {ids}); for (const id of ids) { const {name} = id.node; @@ -276,31 +279,52 @@ export default (): PluginObj<{ jestObjExpr.replaceWith( callExpression(this.declareJestObjGetterIdentifier(), []), ); - exprStmt.setData('_jestShouldHoist', true); } }, }, // in `post` to make sure we come after an import transform and can unshift above the `require`s post({path: program}: {path: NodePath}) { + const self = this; + visitBlock(program); program.traverse({ - ExpressionStatement: exprStmt => { - if (exprStmt.getData('_jestShouldHoist')) { - const prevSiblings = exprStmt.getAllPrevSiblings(); - const unhoistedSiblings = prevSiblings - .reverse() - .filter( - s => - !s.getData('_jestShouldHoist') && !s.getData('_jestWasHoisted'), - ); - const exprNode = exprStmt.node; - if (unhoistedSiblings.length) { - const hoisted = unhoistedSiblings[0].insertBefore(clone(exprNode)); - hoisted[0].setData('_jestWasHoisted', true); - exprStmt.remove(); + BlockStatement: visitBlock, + }); + + 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', + emptyStatement(), + ); + block.traverse({ + CallExpression: visitCallExpr, + // 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(); + + function visitCallExpr(callExpr: NodePath) { + const { + node: {callee}, + } = callExpr; + if ( + isIdentifier(callee) && + callee.name === self.jestObjGetterIdentifier?.name + ) { + const mockStmt = callExpr.getStatementParent(); + + if (mockStmt) { + const mockStmtNode = mockStmt.node; + const mockStmtParent = mockStmt.parentPath; + if (mockStmtParent.isBlock()) { + mockStmt.remove(); + firstNonHoistedStatementOfBlock.insertBefore(mockStmtNode); + } } } - }, - }); + } + } }, }); /* eslint-enable */ From 95601040a8cf52a91533dcd118889906ba4ef691 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 28 Oct 2020 21:14:46 +0100 Subject: [PATCH 11/12] merge changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9869e1d64cc..754f602dd2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixes +- `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10536](https://github.com/facebook/jest/pull/10536)) - `[jest-config]` Fix bug introduced in watch mode by PR[#10678](https://github.com/facebook/jest/pull/10678/files#r511037803) ([#10692](https://github.com/facebook/jest/pull/10692)) - `[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)) @@ -91,7 +92,6 @@ ### Fixes -- `[babel-plugin-jest-hoist]` Preserve order of hoisted mock nodes within containing block ([#10536](https://github.com/facebook/jest/pull/10536)) - `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) - `[jest-console]` Add `Console` constructor to `console` object ([#10502](https://github.com/facebook/jest/pull/10502)) - `[jest-globals]` Fix lifecycle hook function types ([#10480](https://github.com/facebook/jest/pull/10480)) From 4d1f19cb70e7a65ce2ee5cf30c9b80987c9bd7d5 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 28 Oct 2020 21:20:15 +0100 Subject: [PATCH 12/12] test correction 2 --- e2e/babel-plugin-jest-hoist/__test_modules__/g.js | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 e2e/babel-plugin-jest-hoist/__test_modules__/g.js diff --git a/e2e/babel-plugin-jest-hoist/__test_modules__/g.js b/e2e/babel-plugin-jest-hoist/__test_modules__/g.js new file mode 100644 index 000000000000..9f271ead5ed7 --- /dev/null +++ b/e2e/babel-plugin-jest-hoist/__test_modules__/g.js @@ -0,0 +1,8 @@ +/** + * 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 () => 'unmocked';