From 85783549186a36e052608228c94a9955002d9317 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 28 Nov 2022 15:58:20 -0800 Subject: [PATCH 1/2] Remove the Jest postinstall step. --- .../remove-postinstall_2022-11-28-23-57.json | 10 ++ .../includes/jestScriptTransformerPatch.js | 107 ------------------ heft-plugins/heft-jest-plugin/package.json | 3 +- 3 files changed, 11 insertions(+), 109 deletions(-) create mode 100644 common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json delete mode 100644 heft-plugins/heft-jest-plugin/includes/jestScriptTransformerPatch.js diff --git a/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json b/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json new file mode 100644 index 00000000000..381308bee50 --- /dev/null +++ b/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft-jest-plugin", + "comment": "Remove a postinstall step that patches Jest in-place. This is better achieved with a PNPM patch.", + "type": "minor" + } + ], + "packageName": "@rushstack/heft-jest-plugin" +} diff --git a/heft-plugins/heft-jest-plugin/includes/jestScriptTransformerPatch.js b/heft-plugins/heft-jest-plugin/includes/jestScriptTransformerPatch.js deleted file mode 100644 index 33584e1f271..00000000000 --- a/heft-plugins/heft-jest-plugin/includes/jestScriptTransformerPatch.js +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -const path = require('path'); -const fs = require('fs'); - -// This patch is to disable cache reads/writes in Jest. Cache reads/writes add overhead I/O to running Jest -// with the heft-jest-plugin, since cache files for the heft typescript jest transformer simply read a file -// from disk and feed it to Jest. In addition, cache interaction has lead to some issues with Jest in the -// past, such as a race condition when attempting to rename the target cache file (see: -// https://github.com/facebook/jest/issues/4444). Passing '--no-cache' to Jest simply tells Jest to not read -// the produced cache files, but does nothing to prevent writing of these files. This patch disables both -// reading and writing of cache files. - -const patchName = path.basename(__filename); -const HEFT_JEST_DISABLE_CACHE_ENV_VARIABLE = 'HEFT_JEST_DISABLE_CACHE'; - -function applyPatch() { - try { - let contextFolder = __dirname; - // Resolve the "@jest/core" package relative to heft-jest-plugin - contextFolder = path.dirname(require.resolve('@jest/core/package.json', { paths: [contextFolder] })); - // Resolve the "@jest/transform" package relative to the @jest/core package - contextFolder = path.dirname(require.resolve('@jest/transform/package.json', { paths: [contextFolder] })); - // Resolve the ScriptTransformer module in the @jest/transform package - const scriptTransformerFilePath = require.resolve('./build/ScriptTransformer', { - paths: [contextFolder] - }); - - // Patch the file contents - patchScriptTransformer(scriptTransformerFilePath); - } catch (e) { - console.error(); - console.error(`ERROR: ${patchName} failed to patch the "@jest/transform" package:`); - console.error(e.toString()); - console.error(); - - throw e; - } -} - -function patchScriptTransformer(scriptPath) { - // This patch is going to be very specific to the version of Jest that we are using. - // This is intentional, because we want to make sure that we don't accidentally break - // future versions of Jest that might have a different implementation. - // - // We will replace the existing implementation of the method to no-op. - let scriptContent = fs.readFileSync(scriptPath, { encoding: 'utf8' }); - const functionsToReplace = ['readCacheFile', 'writeCacheFile']; - - for (const functionName of functionsToReplace) { - const originalFunctionName = `${functionName}Original`; - - // First, attempt to remove the existing patch, if one is present - let match = scriptContent.match(new RegExp(`^\\s*const ${originalFunctionName} =`, 'm')); - let originalFunctionContent; - if (match) { - const startIndex = match.index; - const endIndex = scriptContent.indexOf('};', startIndex) + 2; - originalFunctionContent = scriptContent.slice(startIndex, endIndex); - scriptContent = scriptContent.slice(0, startIndex) + scriptContent.slice(endIndex); - } - - // Then, attempt to find the function to patch - match = scriptContent.match(new RegExp(`^\\s*const ${functionName} =`, 'm')); - if (!match) { - throw new Error( - `The ${JSON.stringify(functionName)} function was not found in the file ${JSON.stringify(scriptPath)}` - ); - } - - const startIndex = match.index; - const endIndex = scriptContent.indexOf('};', startIndex) + 2; - - // If we already have the original function content, no need to extract it again - if (originalFunctionContent === undefined) { - originalFunctionContent = scriptContent - .slice(startIndex, endIndex) - .replace(`const ${functionName} =`, `const ${originalFunctionName} =`); - } - - scriptContent = - scriptContent.slice(0, startIndex) + - `${originalFunctionContent}\n` + - `const ${functionName} = (...args) => {\n` + - ` // Patched by @rushstack/heft-jest-plugin. For more information, see:\n` + - ` // https://github.com/microsoft/rushstack/pull/3606\n` + - ` if (process.env['${HEFT_JEST_DISABLE_CACHE_ENV_VARIABLE}']) {\n` + - ` return;\n` + - ` }\n` + - ` return ${originalFunctionName}(...args);\n` + - '};' + - scriptContent.slice(endIndex); - } - - // Delete instead of modifying the file. We want to break any hardlinks/symlinks - // used by the package manager to link to the original file - fs.unlinkSync(scriptPath); - fs.writeFileSync(scriptPath, scriptContent); -} - -if (typeof jest !== 'undefined' || process.env.JEST_WORKER_ID) { - // This patch is incompatible with Jest's proprietary require() implementation - console.log(`\nJEST ENVIRONMENT DETECTED - Skipping Heft's ${patchName}\n`); -} else { - applyPatch(); -} diff --git a/heft-plugins/heft-jest-plugin/package.json b/heft-plugins/heft-jest-plugin/package.json index bb80b7d891b..8c0d441887a 100644 --- a/heft-plugins/heft-jest-plugin/package.json +++ b/heft-plugins/heft-jest-plugin/package.json @@ -15,8 +15,7 @@ "build": "heft build --clean", "start": "heft test --clean --watch", "_phase:build": "heft build --clean", - "_phase:test": "heft test --no-build", - "postinstall": "node ./includes/jestScriptTransformerPatch.js" + "_phase:test": "heft test --no-build" }, "peerDependencies": { "@rushstack/heft": "^0.48.8" From fe5ce8c38f2b5d3504eb0a8f3409ec714b6b237f Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 28 Nov 2022 16:02:35 -0800 Subject: [PATCH 2/2] Rush change. --- .../heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json b/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json index 381308bee50..af2e310484f 100644 --- a/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json +++ b/common/changes/@rushstack/heft-jest-plugin/remove-postinstall_2022-11-28-23-57.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/heft-jest-plugin", - "comment": "Remove a postinstall step that patches Jest in-place. This is better achieved with a PNPM patch.", + "comment": "Remove a postinstall step that patches Jest in-place. This is better achieved with a PNPM patch. See https://github.com/microsoft/rushstack/pull/3790 for more information.", "type": "minor" } ],