[compiler][rfc] Enable more validations in playground.

This is mostly to kick off conversation, i think we should go with a modified version of the implemented approach that i'll describe here.

The playground currently serves two roles. The primary one we think about is for verifying compiler output. We use it for this sometimes, and developers frequently use it for this, including to send us repros if they have a potential bug. The second mode is to help developers learn about React. Part of that includes learning how to use React correctly — where it's helpful to see feedback about problematic code — and also to understand what kind of tools we provide compared to other frameworks, to make an informed choice about what tools they want to use.

Currently we primarily think about the first role, but I think we should emphasize the second more. In this PR i'm doing the worst of both: enabling all the validations used by both the compiler and the linter by default. This means that code that would actually compile can fail with validations, which isn't great.

What I think we should actually do is compile twice, one in "compilation" mode and once in "linter" mode, and combine the results as follows:
* If "compilation" mode succeeds, show the compiled output _and_ any linter errors.
* If "compilation" mode fails, show only the compilation mode failures.

We should also distinguish which case it is when we show errors: "Compilation succeeded", "Compilation succeeded with linter errors", "Compilation failed".

This lets developers continue to verify compiler output, while also turning the playground into a much more useful tool for learning React. Thoughts?

compiler/apps/playground/components/Editor/EditorImpl.tsx

@@ -11,6 +11,7 @@ import * as t from '@babel/types';
 import BabelPluginReactCompiler, {
   CompilerError,
   CompilerErrorDetail,
+  CompilerDiagnostic,
   Effect,
   ErrorSeverity,
   parseConfigPragmaForTests,
@@ -141,10 +142,13 @@ const COMMON_HOOKS: Array<[string, Hook]> = [
   ],
 ];
 
-function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
+function compile(
+  source: string,
+  mode: 'compiler' | 'linter',
+): [CompilerOutput, 'flow' | 'typescript'] {
   const results = new Map<string, Array<PrintedCompilerPipelineValue>>();
   const error = new CompilerError();
-  const otherErrors: Array<CompilerErrorDetail> = [];
+  const otherErrors: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
   const upsert: (result: PrintedCompilerPipelineValue) => void = result => {
     const entry = results.get(result.name);
     if (Array.isArray(entry)) {
@@ -203,6 +207,22 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
     };
     const parsedOptions = parseConfigPragmaForTests(pragma, {
       compilationMode: 'infer',
+      environment:
+        mode === 'linter'
+          ? {
+              // enabled in compiler
+              validateRefAccessDuringRender: false,
+              // enabled in linter
+              validateNoSetStateInRender: true,
+              validateNoSetStateInEffects: true,
+              validateNoJSXInTryStatements: true,
+              validateNoImpureFunctionsInRender: true,
+              validateStaticComponents: true,
+              validateNoFreezingKnownMutableFunctions: true,
+            }
+          : {
+              /* use defaults for compiler mode */
+            },
     });
     const opts: PluginOptions = parsePluginOptions({
       ...parsedOptions,
@@ -214,7 +234,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
         debugLogIRs: logIR,
         logEvent: (_filename: string | null, event: LoggerEvent) => {
           if (event.kind === 'CompileError') {
-            otherErrors.push(new CompilerErrorDetail(event.detail));
+            otherErrors.push(event.detail);
           }
         },
       },
@@ -226,7 +246,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
      * (i.e. object shape that is not CompilerError)
      */
     if (err instanceof CompilerError && err.details.length > 0) {
-      error.details.push(...err.details);
+      error.merge(err);
     } else {
       /**
        * Handle unexpected failures by logging (to get a stack trace)
@@ -245,12 +265,15 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
   }
   // Only include logger errors if there weren't other errors
   if (!error.hasErrors() && otherErrors.length !== 0) {
-    otherErrors.forEach(e => error.push(e));
+    otherErrors.forEach(e => error.details.push(e));
   }
   if (error.hasErrors()) {
-    return [{kind: 'err', results, error: error}, language];
+    return [{kind: 'err', results, error}, language];
   }
-  return [{kind: 'ok', results, transformOutput}, language];
+  return [
+    {kind: 'ok', results, transformOutput, errors: error.details},
+    language,
+  ];
 }
 
 export default function Editor(): JSX.Element {
@@ -259,7 +282,11 @@ export default function Editor(): JSX.Element {
   const dispatchStore = useStoreDispatch();
   const {enqueueSnackbar} = useSnackbar();
   const [compilerOutput, language] = useMemo(
-    () => compile(deferredStore.source),
+    () => compile(deferredStore.source, 'compiler'),
+    [deferredStore.source],
+  );
+  const [linterOutput] = useMemo(
+    () => compile(deferredStore.source, 'linter'),
     [deferredStore.source],
   );
 
@@ -285,19 +312,26 @@ export default function Editor(): JSX.Element {
     });
   });
 
+  let mergedOutput: CompilerOutput;
+  let errors: Array<CompilerErrorDetail | CompilerDiagnostic>;
+  if (compilerOutput.kind === 'ok') {
+    errors = linterOutput.kind === 'ok' ? [] : linterOutput.error.details;
+    mergedOutput = {
+      ...compilerOutput,
+      errors,
+    };
+  } else {
+    mergedOutput = compilerOutput;
+    errors = compilerOutput.error.details;
+  }
   return (
     <>
       <div className="relative flex basis top-14">
         <div className={clsx('relative sm:basis-1/4')}>
-          <Input
-            language={language}
-            errors={
-              compilerOutput.kind === 'err' ? compilerOutput.error.details : []
-            }
-          />
+          <Input language={language} errors={errors} />
         </div>
         <div className={clsx('flex sm:flex flex-wrap')}>
-          <Output store={deferredStore} compilerOutput={compilerOutput} />
+          <Output store={deferredStore} compilerOutput={mergedOutput} />
         </div>
       </div>
     </>

compiler/apps/playground/components/Editor/Input.tsx

@@ -36,13 +36,18 @@ export default function Input({errors, language}: Props): JSX.Element {
     const uri = monaco.Uri.parse(`file:///index.js`);
     const model = monaco.editor.getModel(uri);
     invariant(model, 'Model must exist for the selected input file.');
-    renderReactCompilerMarkers({monaco, model, details: errors});
+    renderReactCompilerMarkers({
+      monaco,
+      model,
+      details: errors,
+      source: store.source,
+    });
     /**
      * N.B. that `tabSize` is a model property, not an editor property.
      * So, the tab size has to be set per model.
      */
     model.updateOptions({tabSize: 2});
-  }, [monaco, errors]);
+  }, [monaco, errors, store.source]);
 
   useEffect(() => {
     /**

compiler/apps/playground/components/Editor/Output.tsx

@@ -11,7 +11,11 @@ import {
   InformationCircleIcon,
 } from '@heroicons/react/outline';
 import MonacoEditor, {DiffEditor} from '@monaco-editor/react';
-import {type CompilerError} from 'babel-plugin-react-compiler';
+import {
+  CompilerErrorDetail,
+  CompilerDiagnostic,
+  type CompilerError,
+} from 'babel-plugin-react-compiler';
 import parserBabel from 'prettier/plugins/babel';
 import * as prettierPluginEstree from 'prettier/plugins/estree';
 import * as prettier from 'prettier/standalone';
@@ -44,6 +48,7 @@ export type CompilerOutput =
       kind: 'ok';
       transformOutput: CompilerTransformOutput;
       results: Map<string, Array<PrintedCompilerPipelineValue>>;
+      errors: Array<CompilerErrorDetail | CompilerDiagnostic>;
     }
   | {
       kind: 'err';
@@ -123,10 +128,36 @@ async function tabify(
       parser: transformOutput.language === 'flow' ? 'babel-flow' : 'babel-ts',
       plugins: [parserBabel, prettierPluginEstree],
     });
+
+    let output: string;
+    let language: string;
+    if (compilerOutput.errors.length === 0) {
+      output = code;
+      language = 'javascript';
+    } else {
+      language = 'markdown';
+      output = `
+# Output
+
+React Compiler compiled this function sucessfully, but there are lint errors that indicate potential issues with the original code.
+
+## ${compilerOutput.errors.length} Lint Errors
+
+${compilerOutput.errors.map(e => e.printErrorMessage(source, {eslint: false})).join('\n\n')}
+
+## Output
+
+\`\`\`js
+${code}
+\`\`\`
+`.trim();
+    }
+
     reorderedTabs.set(
-      'JS',
+      'Output',
       <TextTabContent
-        output={code}
+        output={output}
+        language={language}
         diff={null}
         showInfoPanel={false}></TextTabContent>,
     );
@@ -142,6 +173,18 @@ async function tabify(
         </>,
       );
     }
+  } else if (compilerOutput.kind === 'err') {
+    const errors = compilerOutput.error.printErrorMessage(source, {
+      eslint: false,
+    });
+    reorderedTabs.set(
+      'Output',
+      <TextTabContent
+        output={errors}
+        language="plaintext"
+        diff={null}
+        showInfoPanel={false}></TextTabContent>,
+    );
   }
   tabs.forEach((tab, name) => {
     reorderedTabs.set(name, tab);
@@ -162,17 +205,32 @@ function getSourceMapUrl(code: string, map: string): string | null {
 }
 
 function Output({store, compilerOutput}: Props): JSX.Element {
-  const [tabsOpen, setTabsOpen] = useState<Set<string>>(() => new Set(['JS']));
+  const [tabsOpen, setTabsOpen] = useState<Set<string>>(
+    () => new Set(['Output']),
+  );
   const [tabs, setTabs] = useState<Map<string, React.ReactNode>>(
     () => new Map(),
   );
+
+  /*
+   * Update the active tab back to the output or errors tab when the compilation state
+   * changes between success/failure.
+   */
+  const [previousOutputKind, setPreviousOutputKind] = useState(
+    compilerOutput.kind,
+  );
+  if (compilerOutput.kind !== previousOutputKind) {
+    setPreviousOutputKind(compilerOutput.kind);
+    setTabsOpen(new Set(['Output']));
+  }
+
   useEffect(() => {
     tabify(store.source, compilerOutput).then(tabs => {
       setTabs(tabs);
     });
   }, [store.source, compilerOutput]);
 
-  const changedPasses: Set<string> = new Set(['JS', 'HIR']); // Initial and final passes should always be bold
+  const changedPasses: Set<string> = new Set(['Output', 'HIR']); // Initial and final passes should always be bold
   let lastResult: string = '';
   for (const [passName, results] of compilerOutput.results) {
     for (const result of results) {
@@ -196,20 +254,6 @@ function Output({store, compilerOutput}: Props): JSX.Element {
         tabs={tabs}
         changedPasses={changedPasses}
       />
-      {compilerOutput.kind === 'err' ? (
-        <div
-          className="flex flex-wrap absolute bottom-0 bg-white grow border-y border-grey-200 transition-all ease-in"
-          style={{width: 'calc(100vw - 650px)'}}>
-          <div className="w-full p-4 basis-full border-b">
-            <h2>COMPILER ERRORS</h2>
-          </div>
-          <pre
-            className="p-4 basis-full text-red-600 overflow-y-scroll whitespace-pre-wrap"
-            style={{width: 'calc(100vw - 650px)', height: '150px'}}>
-            <code>{compilerOutput.error.toString()}</code>
-          </pre>
-        </div>
-      ) : null}
     </>
   );
 }
@@ -218,10 +262,12 @@ function TextTabContent({
   output,
   diff,
   showInfoPanel,
+  language,
 }: {
   output: string;
   diff: string | null;
   showInfoPanel: boolean;
+  language: string;
 }): JSX.Element {
   const [diffMode, setDiffMode] = useState(false);
   return (
@@ -272,7 +318,7 @@ function TextTabContent({
         />
       ) : (
         <MonacoEditor
-          defaultLanguage="javascript"
+          language={language ?? 'javascript'}
           value={output}
           options={{
             ...monacoOptions,

compiler/apps/playground/lib/reactCompilerMonacoDiagnostics.ts

@@ -6,7 +6,11 @@
  */
 
 import {Monaco} from '@monaco-editor/react';
-import {CompilerErrorDetail, ErrorSeverity} from 'babel-plugin-react-compiler';
+import {
+  CompilerDiagnostic,
+  CompilerErrorDetail,
+  ErrorSeverity,
+} from 'babel-plugin-react-compiler';
 import {MarkerSeverity, type editor} from 'monaco-editor';
 
 function mapReactCompilerSeverityToMonaco(
@@ -22,38 +26,46 @@ function mapReactCompilerSeverityToMonaco(
 }
 
 function mapReactCompilerDiagnosticToMonacoMarker(
-  detail: CompilerErrorDetail,
+  detail: CompilerErrorDetail | CompilerDiagnostic,
   monaco: Monaco,
+  source: string,
 ): editor.IMarkerData | null {
-  if (detail.loc == null || typeof detail.loc === 'symbol') {
+  const loc = detail.primaryLocation();
+  if (loc == null || typeof loc === 'symbol') {
     return null;
   }
   const severity = mapReactCompilerSeverityToMonaco(detail.severity, monaco);
-  let message = detail.printErrorMessage();
+  let message = detail.printErrorMessage(source, {eslint: true});
   return {
     severity,
     message,
-    startLineNumber: detail.loc.start.line,
-    startColumn: detail.loc.start.column + 1,
-    endLineNumber: detail.loc.end.line,
-    endColumn: detail.loc.end.column + 1,
+    startLineNumber: loc.start.line,
+    startColumn: loc.start.column + 1,
+    endLineNumber: loc.end.line,
+    endColumn: loc.end.column + 1,
   };
 }
 
 type ReactCompilerMarkerConfig = {
   monaco: Monaco;
   model: editor.ITextModel;
-  details: Array<CompilerErrorDetail>;
+  details: Array<CompilerErrorDetail | CompilerDiagnostic>;
+  source: string;
 };
 let decorations: Array<string> = [];
 export function renderReactCompilerMarkers({
   monaco,
   model,
   details,
+  source,
 }: ReactCompilerMarkerConfig): void {
   const markers: Array<editor.IMarkerData> = [];
   for (const detail of details) {
-    const marker = mapReactCompilerDiagnosticToMonacoMarker(detail, monaco);
+    const marker = mapReactCompilerDiagnosticToMonacoMarker(
+      detail,
+      monaco,
+      source,
+    );
     if (marker == null) {
       continue;
     }

compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts

@@ -84,9 +84,7 @@ export default function BabelPluginReactCompiler(
             }
           } catch (e) {
             if (e instanceof CompilerError) {
-              throw new Error(
-                e.printErrorMessage(pass.file.code, {eslint: false}),
-              );
+              throw e.withPrintedMessage(pass.file.code, {eslint: false});
             }
             throw e;
           }

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

@@ -262,6 +262,7 @@ export class CompilerErrorDetail {
 
 export class CompilerError extends Error {
   details: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
+  printedMessage: string | null = null;
 
   static invariant(
     condition: unknown,
@@ -347,18 +348,29 @@ export class CompilerError extends Error {
   }
 
   override get message(): string {
-    return this.toString();
+    return this.printedMessage ?? this.toString();
   }
 
   override set message(_message: string) {}
 
   override toString(): string {
+    if (this.printedMessage) {
+      return this.printedMessage;
+    }
     if (Array.isArray(this.details)) {
       return this.details.map(detail => detail.toString()).join('\n\n');
     }
     return this.name;
   }
 
+  withPrintedMessage(
+    source: string,
+    options: PrintErrorMessageOptions,
+  ): CompilerError {
+    this.printedMessage = this.printErrorMessage(source, options);
+    return this;
+  }
+
   printErrorMessage(source: string, options: PrintErrorMessageOptions): string {
     if (options.eslint && this.details.length === 1) {
       return this.details[0].printErrorMessage(source, options);

compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts

@@ -113,8 +113,13 @@ function* splitPragma(
  */
 function parseConfigPragmaEnvironmentForTest(
   pragma: string,
+  defaultConfig: PartialEnvironmentConfig,
 ): EnvironmentConfig {
-  const maybeConfig: Partial<Record<keyof EnvironmentConfig, unknown>> = {};
+  // throw early if the defaults are invalid
+  EnvironmentConfigSchema.parse(defaultConfig);
+
+  const maybeConfig: Partial<Record<keyof EnvironmentConfig, unknown>> =
+    defaultConfig;
 
   for (const {key, value: val} of splitPragma(pragma)) {
     if (!hasOwnProperty(EnvironmentConfigSchema.shape, key)) {
@@ -174,9 +179,13 @@ export function parseConfigPragmaForTests(
   pragma: string,
   defaults: {
     compilationMode: CompilationMode;
+    environment?: PartialEnvironmentConfig;
   },
 ): PluginOptions {
-  const environment = parseConfigPragmaEnvironmentForTest(pragma);
+  const environment = parseConfigPragmaEnvironmentForTest(
+    pragma,
+    defaults.environment ?? {},
+  );
   const options: Record<keyof PluginOptions, unknown> = {
     ...defaultOptions,
     panicThreshold: 'all_errors',