fix(hooks): gate in-process hooks with the TypeScript parser instead of esprima

The in-process gate parsed every .js/.cjs hook with esprima, which is
abandoned and pinned to ES2017. A hook using `??`, `?.` or class fields
made the parse throw, so the gate fell back to spawning the file as a
child process, where the exported hook function is never invoked - the
hook silently did nothing and reported exit code 0.

The same silent no-op hit hooks written as ES modules and transpiled to
CommonJS, since tsc emits `exports.default = ...` and the walker only
recognised `module.exports = ...`.

The gate now parses with the TypeScript compiler API (already a runtime
dependency, required lazily so startup is unaffected), accepts both
`module.exports` and `exports.default` top-level assignments, and the
in-process path unwraps a function-valued `.default`. Modules that reach
the in-process path without exporting a function now warn instead of
crashing on an undefined `$inject`.

esprima is dropped from the dependencies.

Author: edusperoni

lib/common/services/hooks-service.ts

@@ -213,7 +213,19 @@ export class HooksService implements IHooksService {
 				const { default: hookFn } = await import(hook.fullPath);
 				hookEntryPoint = hookFn;
 			} else {
-				hookEntryPoint = require(hook.fullPath);
+				const hookModule = require(hook.fullPath);
+				// transpiled ES modules expose the hook as `exports.default`.
+				hookEntryPoint =
+					hookModule && typeof hookModule.default === "function"
+						? hookModule.default
+						: hookModule;
+			}
+
+			if (typeof hookEntryPoint !== "function") {
+				this.$logger.warn(
+					`${hook.fullPath} will NOT be executed because it does not export a function.`,
+				);
+				return;
 			}
 
 			this.$logger.trace(`Validating ${hookName} arguments.`);
@@ -462,33 +474,44 @@ export class HooksService implements IHooksService {
 
 	private shouldExecuteInProcess(scriptSource: string): boolean {
 		try {
-			const esprima = require("esprima");
-			const ast = esprima.parse(scriptSource);
-
-			let inproc = false;
-			ast.body.forEach((statement: any) => {
-				if (
-					statement.type !== "ExpressionStatement" ||
-					statement.expression.type !== "AssignmentExpression"
-				) {
-					return;
+			// required lazily so that CLI startup does not pay the cost of loading
+			// the TypeScript compiler, which is only needed when a hook runs.
+			const ts = require("typescript");
+			const sourceFile = ts.createSourceFile(
+				"hook.js",
+				scriptSource,
+				ts.ScriptTarget.Latest,
+				/* setParentNodes */ false,
+				ts.ScriptKind.JS,
+			);
+
+			const isExportsTarget = (node: any): boolean => {
+				if (!ts.isPropertyAccessExpression(node)) {
+					return false;
 				}
 
-				const left = statement.expression.left;
-				if (
-					left.type === "MemberExpression" &&
-					left.object &&
-					left.object.type === "Identifier" &&
-					left.object.name === "module" &&
-					left.property &&
-					left.property.type === "Identifier" &&
-					left.property.name === "exports"
-				) {
-					inproc = true;
+				if (!ts.isIdentifier(node.expression)) {
+					return false;
 				}
-			});
 
-			return inproc;
+				const object = node.expression.text;
+				const property = node.name.text;
+
+				return (
+					(object === "module" && property === "exports") ||
+					(object === "exports" && property === "default")
+				);
+			};
+
+			return sourceFile.statements.some((statement: any) => {
+				return (
+					ts.isExpressionStatement(statement) &&
+					ts.isBinaryExpression(statement.expression) &&
+					statement.expression.operatorToken.kind ===
+						ts.SyntaxKind.EqualsToken &&
+					isExportsTarget(statement.expression.left)
+				);
+			});
 		} catch (err) {
 			return false;
 		}

lib/common/test/unit-tests/services/hook-service.ts

@@ -143,6 +143,157 @@ describe("hooks-service", () => {
 		);
 	});
 
+	it("should run hooks using syntax newer than ES2017", async () => {
+		const projectName = "projectDirectory";
+		const projectPath = mkdtempSync(path.join(tmpdir(), `${projectName}-`));
+
+		const testInjector = createTestInjector({ projectDir: projectPath });
+
+		const script = [
+			`class Message {`,
+			`	text = "after-prepare hook is running";`,
+			`}`,
+			`module.exports = function ($logger, hookArgs) {`,
+			`	const message = new Message();`,
+			`	$logger.info(message?.text ?? "fallback");`,
+			`};`,
+		].join("\n");
+
+		fs.mkdirSync(path.join(projectPath, "hooks"));
+		fs.mkdirSync(path.join(projectPath, "hooks/after-prepare"));
+		fs.writeFileSync(
+			path.join(projectPath, "hooks/after-prepare/hook.js"),
+			script,
+		);
+
+		service = testInjector.resolve("$hooksService");
+
+		await service.executeAfterHooks("prepare", { hookArgs: {} });
+
+		assert.equal(
+			testInjector.resolve("$logger").output,
+			"after-prepare hook is running\n",
+		);
+	});
+
+	it("should run hooks transpiled from a default export", async () => {
+		const projectName = "projectDirectory";
+		const projectPath = mkdtempSync(path.join(tmpdir(), `${projectName}-`));
+
+		const testInjector = createTestInjector({ projectDir: projectPath });
+
+		const script = [
+			`"use strict";`,
+			`Object.defineProperty(exports, "__esModule", { value: true });`,
+			`exports.default = function ($logger, hookArgs) {`,
+			`	$logger.info("after-prepare hook is running");`,
+			`};`,
+		].join("\n");
+
+		fs.mkdirSync(path.join(projectPath, "hooks"));
+		fs.mkdirSync(path.join(projectPath, "hooks/after-prepare"));
+		fs.writeFileSync(
+			path.join(projectPath, "hooks/after-prepare/hook.js"),
+			script,
+		);
+
+		service = testInjector.resolve("$hooksService");
+
+		await service.executeAfterHooks("prepare", { hookArgs: {} });
+
+		assert.equal(
+			testInjector.resolve("$logger").output,
+			"after-prepare hook is running\n",
+		);
+	});
+
+	it("should not run in-process hooks that do not export a function", async () => {
+		const projectName = "projectDirectory";
+		const projectPath = mkdtempSync(path.join(tmpdir(), `${projectName}-`));
+
+		const testInjector = createTestInjector({ projectDir: projectPath });
+
+		const script = [`module.exports = { name: "not-a-hook" };`].join("\n");
+
+		fs.mkdirSync(path.join(projectPath, "hooks"));
+		fs.mkdirSync(path.join(projectPath, "hooks/after-prepare"));
+		fs.writeFileSync(
+			path.join(projectPath, "hooks/after-prepare/hook.js"),
+			script,
+		);
+
+		service = testInjector.resolve("$hooksService");
+
+		await service.executeAfterHooks("prepare", { hookArgs: {} });
+
+		expect(testInjector.resolve("$logger").warnOutput).to.have.string(
+			"does not export a function",
+		);
+	});
+
+	describe("in-process detection", () => {
+		const shouldExecuteInProcess = (source: string): boolean => {
+			const testInjector = createTestInjector();
+			const hooksService = testInjector.resolve("$hooksService");
+			return (<any>hooksService).shouldExecuteInProcess(source);
+		};
+
+		it("detects module.exports assignments alongside modern syntax", () => {
+			assert.isTrue(
+				shouldExecuteInProcess(
+					[
+						`class Message { text = "hi"; }`,
+						`module.exports = function ($logger) {`,
+						`	$logger.info(new Message()?.text ?? "fallback");`,
+						`};`,
+					].join("\n"),
+				),
+			);
+		});
+
+		it("detects exports.default assignments", () => {
+			assert.isTrue(
+				shouldExecuteInProcess(
+					[
+						`"use strict";`,
+						`Object.defineProperty(exports, "__esModule", { value: true });`,
+						`exports.default = function ($logger) {};`,
+					].join("\n"),
+				),
+			);
+		});
+
+		it("ignores scripts without an export assignment", () => {
+			assert.isFalse(
+				shouldExecuteInProcess(
+					[
+						`var fs = require("fs");`,
+						`fs.writeFileSync("test.txt", "test");`,
+					].join("\n"),
+				),
+			);
+		});
+
+		it("ignores nested and unrelated assignments", () => {
+			assert.isFalse(
+				shouldExecuteInProcess(
+					[
+						`function register() {`,
+						`	module.exports = function () {};`,
+						`}`,
+						`exports.named = function () {};`,
+						`module.other = function () {};`,
+					].join("\n"),
+				),
+			);
+		});
+
+		it("does not throw on unparseable sources", () => {
+			assert.isFalse(shouldExecuteInProcess(`}{ this is not ((javascript`));
+			assert.isFalse(shouldExecuteInProcess(<any>null));
+		});
+	});
+
 	it("should run non-hook files", async () => {
 		const projectName = "projectDirectory";
 		const projectPath = mkdtempSync(path.join(tmpdir(), `${projectName}-`));

package-lock.json

@@ -55,7 +55,6 @@
     "convert-source-map": "2.0.0",
     "detect-newline": "3.1.0",
     "email-validator": "2.0.4",
-    "esprima": "4.0.1",
     "font-finder": "1.1.0",
     "ios-device-lib": "0.9.5",
     "ios-mobileprovision-finder": "1.2.1",