compiler: add verbose logging to `react-compiler-healthcheck` #29080

Fausto95 · 2024-05-15T21:59:00Z

Summary

This PR adds a verbose option to react-compiler-healthcheck CLI which logs all the compiled components/hooks names and files location.

Fixes: #29078

Test plan

Run the react-compiler-healthcheck CLI with the —-verbose flag inside a react app and you should be able to see the compiled components/hooks names or the files path printed in green.

If there are any files that failed to compile, they will be printed in red.

…ompiled files

react-sizebot · 2024-05-15T22:05:54Z

Comparing: 5e11e7f...a33ece0

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.66 kB	6.66 kB	=	1.82 kB	1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	495.01 kB	495.01 kB	=	88.67 kB	88.68 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.67 kB	6.67 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	499.81 kB	499.81 kB	=	89.36 kB	89.36 kB
facebook-www/ReactDOM-prod.classic.js	=	592.16 kB	592.16 kB	=	104.15 kB	104.15 kB
facebook-www/ReactDOM-prod.modern.js	=	568.39 kB	568.39 kB	=	100.55 kB	100.55 kB
test_utils/ReactAllWarnings.js	Deleted	64.26 kB	0.00 kB	Deleted	16.02 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	64.26 kB	0.00 kB	Deleted	16.02 kB	0.00 kB

Generated by 🚫 dangerJS against a33ece0

Fausto95 · 2024-05-15T23:32:59Z

@poteto, @josephsavona would appreciate if you could review this

josephsavona · 2024-05-16T14:42:42Z

Can you explain more about how you would use this mode? The healthcheck command is intended as a lightweight way to get a general sense of how likely it is that your app will be compatible w the compiler.

Fausto95 · 2024-05-16T14:54:15Z

@josephsavona This mode would tell me the files that compile correctly, this would help me refactor those files and get rid of the manual useMemo & useCallbacks.

This would also help me figure out which files break the rules of React and understand the reason they failed compilation and further investigate.

josephsavona · 2024-05-17T00:38:48Z

Thanks for the context, we'll discuss as a team about where we want to take the tool and whether this fits in. The original idea was just to give folks a quick way to check how easy it would be to adopt the compiler.

Frenie8006 · 2024-05-17T04:44:15Z

Just accept the PR!

wojtekmaj

Final code after applying all of my suggestions:

      for (const compilation of [...SucessfulCompilation, ...ActionableFailures, ...OtherFailures]) {
        const filename = compilation.fnLoc?.filename;

        switch (compilation.kind) {
          case "CompileSuccess": {
            const name = compilation.fnName;
            const isHook = name?.startsWith('use');

            if (name) {
              console.log(
                chalk.green(
                  `Successfully compiled ${isHook ? "hook" : "component" } [${name}](${filename})`
                )
              );
            } else {
              console.log(chalk.green(`Successfully compiled ${compilation.fnLoc?.filename}`));
            }
            break;
          }

          case "CompileError": {
            const { reason, severity, loc } = compilation.detail;

            const lnNo = loc.start?.line;
            const colNo = loc.start?.column;

            const isTodo = severity === ErrorSeverity.Todo;

            console.log(
              chalk[isTodo ? 'yellow' : 'red'](
                `Failed to compile ${
                  filename
                }${
                  lnNo !== undefined ? `:${lnNo}${
                    colNo !== undefined ? `:${colNo}` : ""
                  }` : ""
                }`
              ),
              reason? `\n  Reason: ${isTodo ? 'Unimplemented' : reason}` : ""
            );
            break;
          }
          default:
            break;
        }
      }

wojtekmaj · 2024-05-17T11:58:05Z

compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts

@@ -111,7 +111,7 @@ export default {
    }
  },

-  report(): void {
+  report(verbose: boolean): void {


To make future options easier to implement, consider

Suggested change

report(verbose: boolean): void {

report({ verbose }: { verbose: boolean }): void {

wojtekmaj · 2024-05-17T12:05:33Z

compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts

+        }
+
+        if (compilation.kind === "CompileError") {
+          const reason = compilation.detail.description;


I found description to be null in most cases. For example:

{ kind: 'CompileError', fnLoc: { start: { line: 34, column: 15, index: 993 }, end: { line: 68, column: 1, index: 1956 }, filename: 'src/components/StationFinder/components/StationsMarkers.tsx', identifierName: undefined }, detail: { reason: 'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly', description: null, severity: 'CannotPreserveMemoization', suggestions: null, loc: { start: [Object], end: [Object], filename: 'src/components/StationFinder/components/StationsMarkers.tsx', identifierName: undefined } } }

Also, adding exact error location would be nice.

So something like this could work better:

const { reason, severity, loc } = compilation.detail; const lnNo = loc.start?.line; const colNo = loc.start?.column; const isTodo = severity === ErrorSeverity.Todo; console.log( chalk[isTodo ? 'yellow' : 'red']( `Failed to compile ${ filename }${ lnNo !== undefined ? `:${lnNo}${ colNo !== undefined ? `:${colNo}` : "" }` : "" }` ), reason? `\n Reason: ${isTodo ? 'Unimplemented' : reason}` : "" );

wojtekmaj · 2024-05-17T12:06:44Z

compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts

+      for (const compilation of [...SucessfulCompilation, ...ActionableFailures, ...OtherFailures]) {
+        const filename = compilation.fnLoc?.filename;
+
+        if (compilation.kind === "CompileSuccess") {


Could be a switch()

wojtekmaj · 2024-05-17T12:14:57Z

compiler/packages/react-compiler-healthcheck/src/checks/reactCompiler.ts

+          }
+        }
+
+        if (compilation.kind === "CompileError") {


Certain errors have severity ErrorSeverity.Todo. I don't think it's worth making it all red, but I don't think we should skip this information either. So perhaps make it a different color or something?

guillaumebrunerie · 2024-05-17T15:34:02Z

This would also help me figure out which files break the rules of React and understand the reason they failed compilation and further investigate.

I think that’s what the ESLint plugin is for, rather than the healthcheck tool.

wojtekmaj · 2024-05-18T12:14:21Z

ESLint plugin is nowhere near as informative. It doesn't tell you when React Compiler bails out because of TODOs on RC side, or manual memoization.

PinkaminaDianePie · 2024-05-19T13:51:37Z

Thanks for the context, we'll discuss as a team about where we want to take the tool and whether this fits in. The original idea was just to give folks a quick way to check how easy it would be to adopt the compiler.

how easy

That's why we need a detailed list of files with the exact issues they have. When this tool tells me "issue in 100 components out of 1000" - it gives me no info, I still have 0 clue on how easy will it be to integrate anything. So I have to install eslint plugin, run the scan over my codebase, and eslint tells me that I have an issue not with 100 files, but with 6. What kind of conclusion may I have? Probably only 1 - react compiler is not even close to being production-ready, so I need to wait until the tooling around it becomes more useful. Currently, it feels like this utility is not just useless, but misleading. And until there will be no clear way to check it (like having a path to problematic file, with an eslint error there), people won't trust this tool

Fausto95 · 2024-05-20T13:43:47Z

@wojtekmaj I appreciate the suggestions.
Will apply some of them if the team decides to go with this PR.

facebook-github-bot added the CLA Signed label May 15, 2024

feat: add a verbose option to react-compiler-healthcheck to log all c…

4e30b93

…ompiled files

Fausto95 force-pushed the compiler-healthcheck-verbose branch from d429825 to 4e30b93 Compare May 15, 2024 22:01

Fausto95 force-pushed the compiler-healthcheck-verbose branch 2 times, most recently from ad2698e to 25e02cb Compare May 15, 2024 23:22

feat: log failed compilation

a33ece0

Fausto95 force-pushed the compiler-healthcheck-verbose branch from 25e02cb to a33ece0 Compare May 15, 2024 23:25

Fausto95 changed the title ~~feat: add a verbose option to react-compiler-healthcheck to log all c…~~ compiler: add verbose logging to react-compiler-healthcheck May 16, 2024

Fausto95 mentioned this pull request May 16, 2024

request: [compiler] react-compiler-healthcheck display the list of compiled files. #29078

Open

wojtekmaj reviewed May 17, 2024

View reviewed changes

kirillzyusko mentioned this pull request May 28, 2024

[MEDIUM] Investigate performance improvements of adopting React Compiler Expensify/App#42211

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: add verbose logging to `react-compiler-healthcheck` #29080

compiler: add verbose logging to `react-compiler-healthcheck` #29080

Fausto95 commented May 15, 2024 •

edited

react-sizebot commented May 15, 2024 •

edited

Fausto95 commented May 15, 2024 •

edited

josephsavona commented May 16, 2024

Fausto95 commented May 16, 2024

josephsavona commented May 17, 2024

Frenie8006 commented May 17, 2024

wojtekmaj left a comment •

edited

wojtekmaj May 17, 2024

wojtekmaj May 17, 2024 •

edited

wojtekmaj May 17, 2024

wojtekmaj May 17, 2024

guillaumebrunerie commented May 17, 2024

wojtekmaj commented May 18, 2024

PinkaminaDianePie commented May 19, 2024

Fausto95 commented May 20, 2024

	report(verbose: boolean): void {
	report({ verbose }: { verbose: boolean }): void {

compiler: add verbose logging to react-compiler-healthcheck #29080

Are you sure you want to change the base?

compiler: add verbose logging to react-compiler-healthcheck #29080

Conversation

Fausto95 commented May 15, 2024 • edited

Summary

Test plan

react-sizebot commented May 15, 2024 • edited

Critical size changes

Significant size changes

Fausto95 commented May 15, 2024 • edited

josephsavona commented May 16, 2024

Fausto95 commented May 16, 2024

josephsavona commented May 17, 2024

Frenie8006 commented May 17, 2024

wojtekmaj left a comment • edited

Choose a reason for hiding this comment

wojtekmaj May 17, 2024

Choose a reason for hiding this comment

wojtekmaj May 17, 2024 • edited

Choose a reason for hiding this comment

wojtekmaj May 17, 2024

Choose a reason for hiding this comment

wojtekmaj May 17, 2024

Choose a reason for hiding this comment

guillaumebrunerie commented May 17, 2024

wojtekmaj commented May 18, 2024

PinkaminaDianePie commented May 19, 2024

Fausto95 commented May 20, 2024

compiler: add verbose logging to `react-compiler-healthcheck` #29080

compiler: add verbose logging to `react-compiler-healthcheck` #29080

Fausto95 commented May 15, 2024 •

edited

react-sizebot commented May 15, 2024 •

edited

Fausto95 commented May 15, 2024 •

edited

wojtekmaj left a comment •

edited

wojtekmaj May 17, 2024 •

edited