Add debug logging to inspect what components are transformed by plugin (#442)

Author: andrewiggins

.changeset/twenty-pillows-scream.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-react-transform": patch
+---
+
+Add debug logging to inspect what components are transformed by plugin

packages/react-transform/package.json

@@ -48,6 +48,7 @@
     "@babel/helper-module-imports": "^7.22.5",
     "@babel/helper-plugin-utils": "^7.22.5",
     "@preact/signals-react": "workspace:^1.3.6",
+    "debug": "^4.3.4",
     "use-sync-external-store": "^1.2.0"
   },
   "peerDependencies": {
@@ -59,6 +60,7 @@
     "@types/babel__core": "^7.20.1",
     "@types/babel__helper-module-imports": "^7.18.0",
     "@types/babel__helper-plugin-utils": "^7.10.0",
+    "@types/debug": "^4.1.12",
     "@types/prettier": "^2.7.3",
     "@types/react": "^18.0.18",
     "@types/react-dom": "^18.0.6",

packages/react-transform/src/index.ts

@@ -7,6 +7,8 @@ import {
 	template,
 } from "@babel/core";
 import { isModule, addNamed } from "@babel/helper-module-imports";
+import type { VisitNodeObject } from "@babel/traverse";
+import debug from "debug";
 
 interface PluginArgs {
 	types: typeof BabelTypes;
@@ -23,6 +25,11 @@ const maybeUsesSignal = "maybeUsesSignal";
 const containsJSX = "containsJSX";
 const alreadyTransformed = "alreadyTransformed";
 
+const logger = {
+	transformed: debug("signals:react-transform:transformed"),
+	skipped: debug("signals:react-transform:skipped"),
+};
+
 const get = (pass: PluginPass, name: any) =>
 	pass.get(`${dataNamespace}/${name}`);
 const set = (pass: PluginPass, name: string, v: any) =>
@@ -143,28 +150,11 @@ function getFunctionName(
 	return getFunctionNameFromParent(path.parentPath);
 }
 
-function fnNameStartsWithCapital(
-	path: NodePath<FunctionLike>,
-	filename: string | undefined
-): boolean {
-	const name = getFunctionName(path);
-	if (!name) return false;
-	if (name === DefaultExportSymbol) {
-		return basename(filename)?.match(/^[A-Z]/) != null ?? false;
-	}
-	return name.match(/^[A-Z]/) != null;
+function fnNameStartsWithCapital(name: string | null): boolean {
+	return name?.match(/^[A-Z]/) != null ?? false;
 }
-function fnNameStartsWithUse(
-	path: NodePath<FunctionLike>,
-	filename: string | undefined
-): boolean {
-	const name = getFunctionName(path);
-	if (!name) return false;
-	if (name === DefaultExportSymbol) {
-		return basename(filename)?.match(/^use[A-Z]/) != null ?? false;
-	}
-
-	return name.match(/^use[A-Z]/) != null;
+function fnNameStartsWithUse(name: string | null): boolean {
+	return name?.match(/^use[A-Z]/) != null ?? null;
 }
 
 function hasLeadingComment(path: NodePath, comment: RegExp): boolean {
@@ -236,41 +226,36 @@ function isOptedOutOfSignalTracking(path: NodePath | null): boolean {
 
 function isComponentFunction(
 	path: NodePath<FunctionLike>,
-	filename: string | undefined
+	functionName: string | null
 ): boolean {
 	return (
 		getData(path.scope, containsJSX) === true && // Function contains JSX
-		fnNameStartsWithCapital(path, filename) // Function name indicates it's a component
+		fnNameStartsWithCapital(functionName) // Function name indicates it's a component
 	);
 }
 
-function isCustomHook(
-	path: NodePath<FunctionLike>,
-	filename: string | undefined
-): boolean {
-	return fnNameStartsWithUse(path, filename); // Function name indicates it's a hook
+function isCustomHook(functionName: string | null): boolean {
+	return fnNameStartsWithUse(functionName); // Function name indicates it's a hook
 }
 
 function shouldTransform(
 	path: NodePath<FunctionLike>,
-	filename: string | undefined,
+	functionName: string | null,
 	options: PluginOptions
 ): boolean {
-	if (getData(path, alreadyTransformed) === true) return false;
-
 	// Opt-out takes first precedence
 	if (isOptedOutOfSignalTracking(path)) return false;
 	// Opt-in opts in to transformation regardless of mode
 	if (isOptedIntoSignalTracking(path)) return true;
 
 	if (options.mode === "all") {
-		return isComponentFunction(path, filename);
+		return isComponentFunction(path, functionName);
 	}
 
 	if (options.mode == null || options.mode === "auto") {
 		return (
 			getData(path.scope, maybeUsesSignal) === true && // Function appears to use signals;
-			(isComponentFunction(path, filename) || isCustomHook(path, filename))
+			(isComponentFunction(path, functionName) || isCustomHook(functionName))
 		);
 	}
 
@@ -341,11 +326,11 @@ function transformFunction(
 	t: typeof BabelTypes,
 	options: PluginOptions,
 	path: NodePath<FunctionLike>,
-	filename: string | undefined,
+	functionName: string | null,
 	state: PluginPass
 ) {
 	let newFunction: FunctionLike;
-	if (isCustomHook(path, filename) || options.experimental?.noTryFinally) {
+	if (isCustomHook(functionName) || options.experimental?.noTryFinally) {
 		// For custom hooks, we don't need to wrap the function body in a
 		// try/finally block because later code in the function's render body could
 		// read signals and we want to track and associate those signals with this
@@ -435,10 +420,70 @@ export interface PluginOptions {
 	};
 }
 
+function log(
+	transformed: boolean,
+	path: NodePath<FunctionLike>,
+	functionName: string | null,
+	currentFile: string | undefined
+) {
+	if (!logger.transformed.enabled && !logger.skipped.enabled) return;
+
+	let cwd = "";
+	if (typeof process !== undefined && typeof process.cwd == "function") {
+		cwd = process.cwd().replace(/\\([^ ])/g, "/$1");
+		cwd = cwd.endsWith("/") ? cwd : cwd + "/";
+	}
+
+	const relativePath = currentFile?.replace(cwd, "") ?? "";
+	const lineNum = path.node.loc?.start.line;
+	functionName = functionName ?? "<anonymous>";
+
+	if (transformed) {
+		logger.transformed(`${functionName} (${relativePath}:${lineNum})`);
+	} else {
+		logger.skipped(`${functionName} (${relativePath}:${lineNum}) %o`, {
+			hasSignals: getData(path.scope, maybeUsesSignal) ?? false,
+			hasJSX: getData(path.scope, containsJSX) ?? false,
+		});
+	}
+}
+
+function isComponentLike(
+	path: NodePath<FunctionLike>,
+	functionName: string | null
+): boolean {
+	return (
+		!getData(path, alreadyTransformed) && fnNameStartsWithCapital(functionName)
+	);
+}
+
 export default function signalsTransform(
 	{ types: t }: PluginArgs,
 	options: PluginOptions
 ): PluginObj {
+	// TODO: Consider alternate implementation, where on enter of a function
+	// expression, we run our own manual scan the AST to determine if the
+	// function uses signals and is a component. This manual scan once upon
+	// seeing a function would probably be faster than running an entire
+	// babel pass with plugins on components twice.
+	const visitFunction: VisitNodeObject<PluginPass, FunctionLike> = {
+		exit(path, state) {
+			if (getData(path, alreadyTransformed) === true) return false;
+
+			let functionName = getFunctionName(path);
+			if (functionName === DefaultExportSymbol) {
+				functionName = basename(this.filename) ?? null;
+			}
+
+			if (shouldTransform(path, functionName, state.opts)) {
+				transformFunction(t, state.opts, path, functionName, state);
+				log(true, path, functionName, this.filename);
+			} else if (isComponentLike(path, functionName)) {
+				log(false, path, functionName, this.filename);
+			}
+		},
+	};
+
 	return {
 		name: "@preact/signals-transform",
 		visitor: {
@@ -462,42 +507,10 @@ export default function signalsTransform(
 				},
 			},
 
-			ArrowFunctionExpression: {
-				// TODO: Consider alternate implementation, where on enter of a function
-				// expression, we run our own manual scan the AST to determine if the
-				// function uses signals and is a component. This manual scan once upon
-				// seeing a function would probably be faster than running an entire
-				// babel pass with plugins on components twice.
-				exit(path, state) {
-					if (shouldTransform(path, this.filename, options)) {
-						transformFunction(t, options, path, this.filename, state);
-					}
-				},
-			},
-
-			FunctionExpression: {
-				exit(path, state) {
-					if (shouldTransform(path, this.filename, options)) {
-						transformFunction(t, options, path, this.filename, state);
-					}
-				},
-			},
-
-			FunctionDeclaration: {
-				exit(path, state) {
-					if (shouldTransform(path, this.filename, options)) {
-						transformFunction(t, options, path, this.filename, state);
-					}
-				},
-			},
-
-			ObjectMethod: {
-				exit(path, state) {
-					if (shouldTransform(path, this.filename, options)) {
-						transformFunction(t, options, path, this.filename, state);
-					}
-				},
-			},
+			ArrowFunctionExpression: visitFunction,
+			FunctionExpression: visitFunction,
+			FunctionDeclaration: visitFunction,
+			ObjectMethod: visitFunction,
 
 			MemberExpression(path) {
 				if (isValueMemberExpression(path)) {

packages/react-transform/test/node/index.test.tsx

@@ -19,7 +19,7 @@ import {
 // To help interactively debug a specific test case, add the test ids of the
 // test cases you want to debug to the `debugTestIds` array, e.g. (["258",
 // "259"]). Set to true to debug all tests.
-const DEBUG_TEST_IDS: string[] | true = true;
+const DEBUG_TEST_IDS: string[] | true = [];
 
 const format = (code: string) => prettier.format(code, { parser: "babel" });
 

packages/react/test/browser/updates.test.tsx

@@ -263,17 +263,16 @@ describe("@preact/signals-react updating", () => {
 		it("should update memo'ed component via signals", async () => {
 			const sig = signal("foo");
 
-			function Inner() {
+			function UseMemoTestInner() {
 				const value = sig.value;
 				return <p>{value}</p>;
 			}
 
-			function App() {
-				sig.value;
-				return useMemo(() => <Inner />, []);
+			function UseMemoTestApp() {
+				return useMemo(() => <UseMemoTestInner />, []);
 			}
 
-			await render(<App />);
+			await render(<UseMemoTestApp />);
 			expect(scratch.textContent).to.equal("foo");
 
 			await act(() => {
@@ -326,10 +325,10 @@ describe("@preact/signals-react updating", () => {
 		it("should consistently rerender in strict mode (with memo)", async () => {
 			const sig = signal(-1);
 
-			const Test = memo(() => <p>{sig.value}</p>);
+			const ReactMemoTest = memo(() => <p>{sig.value}</p>);
 			const App = () => (
 				<StrictMode>
-					<Test />
+					<ReactMemoTest />
 				</StrictMode>
 			);
 
@@ -347,7 +346,7 @@ describe("@preact/signals-react updating", () => {
 		it("should render static markup of a component", async () => {
 			const count = signal(0);
 
-			const Test = () => {
+			const StaticMarkupTest = () => {
 				return (
 					<pre>
 						{renderToStaticMarkup(<code>{count}</code>)}
@@ -356,7 +355,7 @@ describe("@preact/signals-react updating", () => {
 				);
 			};
 
-			await render(<Test />);
+			await render(<StaticMarkupTest />);
 			expect(scratch.textContent).to.equal("<code>0</code><code>0</code>");
 
 			for (let i = 0; i < 3; i++) {