[other] Change instrumentation to use an optional gating identifier; record filepath
[other] Change instrumentation to use an optional gating identifier; record filepath Internal rollout currently has a good number of test failures. `enableEmitInstrumentForget` can help developers understand which functions / files they should look at: ``` // input function Foo() { userCode(); // ... } // output function Foo() { if (__DEV__ && inE2eTestMode) { logRender("Foo", "/path/to/filename.js"); } const $ = useMemoCache(...); userCode(); } ```
Mofei Zhang committed
Feb 27, 2024 at 22:06 UTC
a06ded902a9d8da050cbbd1ee162ed2c7d83f828
10 files changed
+90
-37
compiler/apps/playground/components/Editor/EditorImpl.tsx
+1
-1
@@ -176,7 +176,7 @@ function compile(source: string): CompilerOutput {
176
for (const result of run(fn, {
177
...config,
178
customHooks: new Map([...COMMON_HOOKS]),
179
- })) {
179
+ }, null)) {
180
const fnName = fn.node.id?.name ?? null;
181
switch (result.kind) {
182
case "ast": {
compiler/packages/babel-plugin-react-forget/scripts/jest/makeTransform.ts
+1
-1
@@ -168,7 +168,7 @@ function ReactForgetFunctionTransform() {
168
}
169
}
170
171
- const compiled = compile(fn, forgetOptions);
171
+ const compiled = compile(fn, forgetOptions, null);
172
compiledFns.add(compiled);
173
174
const fun = t.functionDeclaration(
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts
+9
-6
@@ -89,7 +89,8 @@ export function* run(
89
func: NodePath<
90
t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
91
>,
92
- config: EnvironmentConfig
92
+ config: EnvironmentConfig,
93
+ filename: string | null
94
): Generator<CompilerPipelineValue, CodegenFunction> {
95
const contextIdentifiers = findContextIdentifiers(func);
96
const env = new Environment(config, contextIdentifiers);
@@ -98,7 +99,7 @@ export function* run(
99
name: "EnvironmentConfig",
100
value: prettyFormat(env.config),
101
};
101
- const ast = yield* runWithEnvironment(func, env);
102
+ const ast = yield* runWithEnvironment(func, env, filename);
103
return ast;
104
}
105
@@ -110,7 +111,8 @@ function* runWithEnvironment(
111
func: NodePath<
112
t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
113
>,
113
- env: Environment
114
+ env: Environment,
115
+ filename: string | null
116
): Generator<CompilerPipelineValue, CodegenFunction> {
117
const hir = lower(func, env).unwrap();
118
yield log({ kind: "hir", name: "HIR", value: hir });
@@ -358,7 +360,7 @@ function* runWithEnvironment(
360
validatePreservedManualMemoization(reactiveFunction);
361
}
362
361
- const ast = codegenFunction(reactiveFunction).unwrap();
363
+ const ast = codegenFunction(reactiveFunction, filename).unwrap();
364
yield log({ kind: "ast", name: "Codegen", value: ast });
365
366
/**
@@ -377,9 +379,10 @@ export function compileFn(
379
func: NodePath<
380
t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
381
>,
380
- config: EnvironmentConfig
382
+ config: EnvironmentConfig,
383
+ filename: string | null
384
): CodegenFunction {
382
- let generator = run(func, config);
385
+ let generator = run(func, config, filename);
386
while (true) {
387
const next = generator.next();
388
if (next.done) {
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts
+19
-6
@@ -252,9 +252,17 @@ export function compileProgram(
252
* TODO(lauren): Remove pass.opts.environment nullcheck once PluginOptions
253
* is validated
254
*/
255
+ if (environment.isErr()) {
256
+ CompilerError.throwInvalidConfig({
257
+ reason: "Error in validating environment config",
258
+ description: environment.unwrapErr().toString(),
259
+ suggestions: null,
260
+ loc: null,
261
+ });
262
+ }
263
const config = environment.unwrap();
264
257
- compiledFn = compileFn(fn, config);
265
+ compiledFn = compileFn(fn, config, pass.filename);
266
pass.opts.logger?.logEvent(pass.filename, {
267
kind: "CompileSuccess",
268
fnLoc: fn.node.loc ?? null,
@@ -319,7 +327,6 @@ export function compileProgram(
327
}
328
329
const externalFunctions: ExternalFunction[] = [];
322
- let instrumentForget: null | ExternalFunction = null;
330
let gating: null | ExternalFunction = null;
331
try {
332
// TODO: check for duplicate import specifiers
@@ -328,11 +335,17 @@ export function compileProgram(
335
externalFunctions.push(gating);
336
}
337
331
- if (options.environment?.enableEmitInstrumentForget != null) {
332
- instrumentForget = tryParseExternalFunction(
333
- options.environment.enableEmitInstrumentForget
338
+ const enableEmitInstrumentForget =
339
+ options.environment?.enableEmitInstrumentForget;
340
+ if (enableEmitInstrumentForget != null) {
341
+ externalFunctions.push(
342
+ tryParseExternalFunction(enableEmitInstrumentForget.fn)
343
);
335
- externalFunctions.push(instrumentForget);
344
+ if (enableEmitInstrumentForget.gating != null) {
345
+ externalFunctions.push(
346
+ tryParseExternalFunction(enableEmitInstrumentForget.gating)
347
+ );
348
+ }
349
}
350
351
if (options.environment?.enableEmitFreeze != null) {
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+13
-5
@@ -45,6 +45,12 @@ export const ExternalFunctionSchema = z.object({
45
// Unique name for the feature flag test condition, eg `isForgetEnabled_ProjectName`
46
importSpecifierName: z.string(),
47
});
48
+
49
+export const InstrumentationSchema = z.object({
50
+ fn: ExternalFunctionSchema,
51
+ gating: ExternalFunctionSchema.nullish(),
52
+});
53
+
54
export type ExternalFunction = z.infer<typeof ExternalFunctionSchema>;
55
56
const HookSchema = z.object({
@@ -254,22 +260,24 @@ const EnvironmentConfigSchema = z.object({
260
* instrumentation function, for components and hooks that Forget compiles.
261
* For example:
262
* instrumentForget: {
257
- * source: 'react-forget-runtime',
258
- * importSpecifierName: 'useRenderCounter',
263
+ * import: {
264
+ * source: 'react-forget-runtime',
265
+ * importSpecifierName: 'useRenderCounter',
266
+ * }
267
* }
268
*
269
* produces:
262
- * import {useRenderCounter} from 'react-forget-runtime-pokes';
270
+ * import {useRenderCounter} from 'react-forget-runtime';
271
*
272
* function Component(props) {
273
* if (__DEV__) {
266
- * useRenderCounter();
274
+ * useRenderCounter("Component", "/filepath/filename.js");
275
* }
276
* // ...
277
* }
278
*
279
*/
272
- enableEmitInstrumentForget: ExternalFunctionSchema.nullish(),
280
+ enableEmitInstrumentForget: InstrumentationSchema.nullish(),
281
282
/**
283
* Enable support for reactive scopes that contain an early return.
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+16
-5
@@ -72,7 +72,8 @@ export type CodegenFunction = {
72
};
73
74
export function codegenFunction(
75
- fn: ReactiveFunction
75
+ fn: ReactiveFunction,
76
+ filename: string | null
77
): Result<CodegenFunction, CompilerError> {
78
const cx = new Context(fn.env, fn.id ?? "[[ anonymous ]]", null);
79
const compileResult = codegenReactiveFunction(cx, fn);
@@ -112,14 +113,24 @@ export function codegenFunction(
113
if (emitInstrumentForget != null && fn.id != null) {
114
/*
115
* Technically, this is a conditional hook call. However, we expect
115
- * __DEV__ and gatingIdentifier to be runtime constants
116
+ * __DEV__ and gating identifier to be runtime constants
117
*/
118
+ let gating: t.Expression;
119
+ if (emitInstrumentForget.gating != null) {
120
+ gating = t.logicalExpression(
121
+ "&&",
122
+ t.identifier("__DEV__"),
123
+ t.identifier(emitInstrumentForget.gating.importSpecifierName)
124
+ );
125
+ } else {
126
+ gating = t.identifier("__DEV__");
127
+ }
128
const test: t.IfStatement = t.ifStatement(
118
- t.identifier("__DEV__"),
129
+ gating,
130
t.expressionStatement(
131
t.callExpression(
121
- t.identifier(emitInstrumentForget.importSpecifierName),
122
- [t.stringLiteral(fn.id)]
132
+ t.identifier(emitInstrumentForget.fn.importSpecifierName),
133
+ [t.stringLiteral(fn.id), t.stringLiteral(filename ?? "")]
134
)
135
)
136
);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/codegen-emit-imports-same-source.expect.md
+7
-2
@@ -13,11 +13,16 @@ function useFoo(props) {
13
## Code
14
15
```javascript
16
-import { useRenderCounter, makeReadOnly } from "react-forget-runtime";
16
+import {
17
+ useRenderCounter,
18
+ shouldInstrument,
19
+ makeReadOnly,
20
+} from "react-forget-runtime";
21
import { unstable_useMemoCache as useMemoCache } from "react"; // @enableEmitFreeze @instrumentForget
22
23
function useFoo(props) {
20
- if (__DEV__) useRenderCounter("useFoo");
24
+ if (__DEV__ && shouldInstrument)
25
+ useRenderCounter("useFoo", "/codegen-emit-imports-same-source.ts");
26
const $ = useMemoCache(2);
27
let t0;
28
if ($[0] !== props.x) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/codegen-instrument-forget-gating-test.expect.md
+5
-3
@@ -24,11 +24,12 @@ function Foo(props) {
24
25
```javascript
26
import { isForgetEnabled_Fixtures } from "ReactForgetFeatureFlag";
27
-import { useRenderCounter } from "react-forget-runtime";
27
+import { useRenderCounter, shouldInstrument } from "react-forget-runtime";
28
import { unstable_useMemoCache as useMemoCache } from "react"; // @instrumentForget @compilationMode(annotation) @gating
29
const Bar = isForgetEnabled_Fixtures()
30
? function Bar(props) {
31
- if (__DEV__) useRenderCounter("Bar");
31
+ if (__DEV__ && shouldInstrument)
32
+ useRenderCounter("Bar", "/codegen-instrument-forget-gating-test.ts");
33
const $ = useMemoCache(2);
34
let t0;
35
if ($[0] !== props.bar) {
@@ -50,7 +51,8 @@ function NoForget(props) {
51
}
52
const Foo = isForgetEnabled_Fixtures()
53
? function Foo(props) {
53
- if (__DEV__) useRenderCounter("Foo");
54
+ if (__DEV__ && shouldInstrument)
55
+ useRenderCounter("Foo", "/codegen-instrument-forget-gating-test.ts");
56
const $ = useMemoCache(2);
57
let t0;
58
if ($[0] !== props.bar) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/codegen-instrument-forget-test.expect.md
+5
-3
@@ -23,11 +23,12 @@ function Foo(props) {
23
## Code
24
25
```javascript
26
-import { useRenderCounter } from "react-forget-runtime";
26
+import { useRenderCounter, shouldInstrument } from "react-forget-runtime";
27
import { unstable_useMemoCache as useMemoCache } from "react"; // @instrumentForget @compilationMode(annotation)
28
29
function Bar(props) {
30
- if (__DEV__) useRenderCounter("Bar");
30
+ if (__DEV__ && shouldInstrument)
31
+ useRenderCounter("Bar", "/codegen-instrument-forget-test.ts");
32
const $ = useMemoCache(2);
33
let t0;
34
if ($[0] !== props.bar) {
@@ -45,7 +46,8 @@ function NoForget(props) {
46
}
47
48
function Foo(props) {
48
- if (__DEV__) useRenderCounter("Foo");
49
+ if (__DEV__ && shouldInstrument)
50
+ useRenderCounter("Foo", "/codegen-instrument-forget-test.ts");
51
const $ = useMemoCache(2);
52
let t0;
53
if ($[0] !== props.bar) {
compiler/packages/snap/src/compiler.ts
+14
-5
@@ -65,8 +65,14 @@ function makePluginOptions(
65
}
66
if (firstLine.includes("@instrumentForget")) {
67
enableEmitInstrumentForget = {
68
- source: "react-forget-runtime",
69
- importSpecifierName: "useRenderCounter",
68
+ fn: {
69
+ source: "react-forget-runtime",
70
+ importSpecifierName: "useRenderCounter",
71
+ },
72
+ gating: {
73
+ source: "react-forget-runtime",
74
+ importSpecifierName: "shouldInstrument",
75
+ },
76
};
77
}
78
if (firstLine.includes("@enableEmitFreeze")) {
@@ -282,6 +288,9 @@ export function transformFixtureInput(
288
const filename =
289
path.basename(fixturePath) + (language === "typescript" ? ".ts" : "");
290
const inputAst = parseInput(input, filename, language);
291
+ // Give babel transforms an absolute path as relative paths get prefixed
292
+ // with `cwd`, which is different across machines
293
+ const virtualFilepath = "/" + filename;
294
295
const presets =
296
language === "typescript"
@@ -292,7 +301,7 @@ export function transformFixtureInput(
301
* Get Forget compiled code
302
*/
303
const forgetResult = transformFromAstSync(inputAst, input, {
295
- filename,
304
+ filename: virtualFilepath,
305
highlightCode: false,
306
retainLines: true,
307
plugins: [
@@ -324,7 +333,7 @@ export function transformFixtureInput(
333
);
334
const result = transformFromAstSync(forgetResult.ast, forgetOutput, {
335
presets,
327
- filename,
336
+ filename: virtualFilepath,
337
});
338
if (result?.code == null) {
339
return {
@@ -348,7 +357,7 @@ export function transformFixtureInput(
357
try {
358
const result = transformFromAstSync(inputAst, input, {
359
presets,
351
- filename,
360
+ filename: virtualFilepath,
361
});
362
363
if (result?.code == null) {