Skip to content

Commit

Permalink
Add debug logging to inspect what components are transformed by plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewiggins committed Nov 16, 2023
1 parent 4c433c3 commit e22f1bc
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 85 deletions.
5 changes: 5 additions & 0 deletions .changeset/twenty-pillows-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals-react-transform": patch
---

Add debug logging to inspect what components are transformed by plugin
2 changes: 2 additions & 0 deletions packages/react-transform/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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",
Expand Down
155 changes: 84 additions & 71 deletions packages/react-transform/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) =>
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: {
Expand All @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-transform/test/node/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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" });

Expand Down
17 changes: 8 additions & 9 deletions packages/react/test/browser/updates.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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>
);

Expand All @@ -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>)}
Expand All @@ -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++) {
Expand Down
20 changes: 16 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit e22f1bc

Please sign in to comment.