From 957547ebb30b8541cfabd21e7051aa0b50f354ff Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 22 Aug 2019 13:22:08 +0200 Subject: [PATCH 1/3] fix: show better errors from babel-plugin-jest-hoist --- packages/babel-plugin-jest-hoist/src/index.ts | 71 +++++++++---------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index 81d0a4cefc65..cf83f2971731 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -7,16 +7,10 @@ */ // Only used for types -// eslint-disable-next-line +/* eslint-disable import/no-extraneous-dependencies */ import {NodePath, Visitor} from '@babel/traverse'; -// eslint-disable-next-line import {Identifier} from '@babel/types'; - -const invariant = (condition: unknown, message: string) => { - if (!condition) { - throw new Error('babel-plugin-jest-hoist: ' + message); - } -}; +/* eslint-enable */ // We allow `jest`, `expect`, `require`, all default Node.js globals and all // ES2015 built-ins to be used inside of a `jest.mock` factory. @@ -93,15 +87,18 @@ const FUNCTIONS: Record< (args: Array) => boolean > = Object.create(null); -FUNCTIONS.mock = (args: Array) => { +FUNCTIONS.mock = args => { if (args.length === 1) { return args[0].isStringLiteral() || args[0].isLiteral(); } else if (args.length === 2 || args.length === 3) { const moduleFactory = args[1]; - invariant( - moduleFactory.isFunction(), - 'The second argument of `jest.mock` must be an inline function.', - ); + + if (!moduleFactory.isFunction()) { + throw moduleFactory.buildCodeFrameError( + 'The second argument of `jest.mock` must be an inline function.\n', + TypeError, + ); + } const ids: Set> = new Set(); const parentScope = moduleFactory.parentPath.scope; @@ -122,23 +119,28 @@ FUNCTIONS.mock = (args: Array) => { } if (!found) { - invariant( + const isAllowedIdentifier = (scope.hasGlobal(name) && WHITELISTED_IDENTIFIERS.has(name)) || - /^mock/i.test(name) || - // Allow istanbul's coverage variable to pass. - /^(?:__)?cov/.test(name), - 'The module factory of `jest.mock()` is not allowed to ' + - 'reference any out-of-scope variables.\n' + - 'Invalid variable access: ' + - name + - '\n' + - 'Whitelisted objects: ' + - Array.from(WHITELISTED_IDENTIFIERS).join(', ') + - '.\n' + - 'Note: This is a precaution to guard against uninitialized mock ' + - 'variables. If it is ensured that the mock is required lazily, ' + - 'variable names prefixed with `mock` (case insensitive) are permitted.', - ); + /^mock/i.test(name) || + // Allow istanbul's coverage variable to pass. + /^(?:__)?cov/.test(name); + + if (!isAllowedIdentifier) { + throw id.buildCodeFrameError( + 'The module factory of `jest.mock()` is not allowed to ' + + 'reference any out-of-scope variables.\n' + + 'Invalid variable access: ' + + name + + '\n' + + 'Whitelisted objects: ' + + Array.from(WHITELISTED_IDENTIFIERS).join(', ') + + '.\n' + + 'Note: This is a precaution to guard against uninitialized mock ' + + 'variables. If it is ensured that the mock is required lazily, ' + + 'variable names prefixed with `mock` (case insensitive) are permitted.\n', + ReferenceError, + ); + } } } @@ -147,13 +149,10 @@ FUNCTIONS.mock = (args: Array) => { return false; }; -FUNCTIONS.unmock = (args: Array) => - args.length === 1 && args[0].isStringLiteral(); -FUNCTIONS.deepUnmock = (args: Array) => - args.length === 1 && args[0].isStringLiteral(); -FUNCTIONS.disableAutomock = FUNCTIONS.enableAutomock = ( - args: Array, -) => args.length === 0; +FUNCTIONS.unmock = args => args.length === 1 && args[0].isStringLiteral(); +FUNCTIONS.deepUnmock = args => args.length === 1 && args[0].isStringLiteral(); +FUNCTIONS.disableAutomock = FUNCTIONS.enableAutomock = args => + args.length === 0; export = () => { const shouldHoistExpression = (expr: NodePath): boolean => { From c848b87a888a5e15eabca0e9875fe48ccd07cdd1 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 22 Aug 2019 13:25:30 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c608af9c29d..a005ccdddb96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Features +- `[babel-plugin-jest-hoist]` Show codeframe on static hoisting issues ([#8865](https://github.com/facebook/jest/pull/8865)) - `[jest-config]` [**BREAKING**] Set default display name color based on runner ([#8689](https://github.com/facebook/jest/pull/8689)) ### Fixes From c1f6cdbf1ea46d65c7bcb4cb33232fbd5939954c Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 22 Aug 2019 14:52:39 +0200 Subject: [PATCH 3/3] sort elements in identifier whitelist to make it easier to read --- packages/babel-plugin-jest-hoist/src/index.ts | 112 +++++++++--------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/packages/babel-plugin-jest-hoist/src/index.ts b/packages/babel-plugin-jest-hoist/src/index.ts index cf83f2971731..d321b13af59a 100644 --- a/packages/babel-plugin-jest-hoist/src/index.ts +++ b/packages/babel-plugin-jest-hoist/src/index.ts @@ -15,62 +15,62 @@ import {Identifier} from '@babel/types'; // 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. -const WHITELISTED_IDENTIFIERS: Set = new Set([ - 'Array', - 'ArrayBuffer', - 'Boolean', - 'DataView', - 'Date', - 'Error', - 'EvalError', - 'Float32Array', - 'Float64Array', - 'Function', - 'Generator', - 'GeneratorFunction', - 'Infinity', - 'Int16Array', - 'Int32Array', - 'Int8Array', - 'InternalError', - 'Intl', - 'JSON', - 'Map', - 'Math', - 'NaN', - 'Number', - 'Object', - 'Promise', - 'Proxy', - 'RangeError', - 'ReferenceError', - 'Reflect', - 'RegExp', - 'Set', - 'String', - 'Symbol', - 'SyntaxError', - 'TypeError', - 'URIError', - 'Uint16Array', - 'Uint32Array', - 'Uint8Array', - 'Uint8ClampedArray', - 'WeakMap', - 'WeakSet', - 'arguments', - 'console', - 'expect', - 'isNaN', - 'jest', - 'parseFloat', - 'parseInt', - 'require', - 'undefined', -]); -Object.getOwnPropertyNames(global).forEach(name => { - WHITELISTED_IDENTIFIERS.add(name); -}); +const WHITELISTED_IDENTIFIERS = new Set( + [ + 'Array', + 'ArrayBuffer', + 'Boolean', + 'DataView', + 'Date', + 'Error', + 'EvalError', + 'Float32Array', + 'Float64Array', + 'Function', + 'Generator', + 'GeneratorFunction', + 'Infinity', + 'Int16Array', + 'Int32Array', + 'Int8Array', + 'InternalError', + 'Intl', + 'JSON', + 'Map', + 'Math', + 'NaN', + 'Number', + 'Object', + 'Promise', + 'Proxy', + 'RangeError', + 'ReferenceError', + 'Reflect', + 'RegExp', + 'Set', + 'String', + 'Symbol', + 'SyntaxError', + 'TypeError', + 'URIError', + 'Uint16Array', + 'Uint32Array', + 'Uint8Array', + 'Uint8ClampedArray', + 'WeakMap', + 'WeakSet', + 'arguments', + 'console', + 'expect', + 'isNaN', + 'jest', + 'parseFloat', + 'parseInt', + 'require', + 'undefined', + ...Object.getOwnPropertyNames(global), + ].sort(), +); const JEST_GLOBAL = {name: 'jest'}; // TODO: Should be Visitor<{ids: Set>}>, but `ReferencedIdentifier` doesn't exist