From 3fbedc5980dbcf8a274159caa966cbf62fff1bd8 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 24 Jul 2025 15:49:20 -0700 Subject: [PATCH] [compiler][rfc] Enable more validations in playground. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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? --- .../components/Editor/EditorImpl.tsx | 55 +++++++++++++++---- .../playground/components/Editor/Output.tsx | 52 +++++++++++++++--- .../src/Utils/TestUtils.ts | 13 ++++- 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 27d2fd79890a..0ced1e54ed76 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -142,7 +142,10 @@ 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>(); const error = new CompilerError(); const otherErrors: Array = []; @@ -204,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, @@ -249,9 +268,12 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { 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 { @@ -260,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], ); @@ -286,19 +312,26 @@ export default function Editor(): JSX.Element { }); }); + let mergedOutput: CompilerOutput; + let errors: Array; + if (compilerOutput.kind === 'ok') { + errors = linterOutput.kind === 'ok' ? [] : linterOutput.error.details; + mergedOutput = { + ...compilerOutput, + errors, + }; + } else { + mergedOutput = compilerOutput; + errors = compilerOutput.error.details; + } return ( <>
- +
- +
diff --git a/compiler/apps/playground/components/Editor/Output.tsx b/compiler/apps/playground/components/Editor/Output.tsx index 36d20a0125e7..0f8fe268ca02 100644 --- a/compiler/apps/playground/components/Editor/Output.tsx +++ b/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>; + errors: Array; } | { 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', , ); @@ -147,9 +178,10 @@ async function tabify( eslint: false, }); reorderedTabs.set( - 'Errors', + 'Output', , ); @@ -173,7 +205,9 @@ function getSourceMapUrl(code: string, map: string): string | null { } function Output({store, compilerOutput}: Props): JSX.Element { - const [tabsOpen, setTabsOpen] = useState>(() => new Set(['JS'])); + const [tabsOpen, setTabsOpen] = useState>( + () => new Set(['Output']), + ); const [tabs, setTabs] = useState>( () => new Map(), ); @@ -187,7 +221,7 @@ function Output({store, compilerOutput}: Props): JSX.Element { ); if (compilerOutput.kind !== previousOutputKind) { setPreviousOutputKind(compilerOutput.kind); - setTabsOpen(new Set([compilerOutput.kind === 'ok' ? 'JS' : 'Errors'])); + setTabsOpen(new Set(['Output'])); } useEffect(() => { @@ -196,7 +230,7 @@ function Output({store, compilerOutput}: Props): JSX.Element { }); }, [store.source, compilerOutput]); - const changedPasses: Set = new Set(['JS', 'HIR']); // Initial and final passes should always be bold + const changedPasses: Set = 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) { @@ -228,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 ( @@ -282,7 +318,7 @@ function TextTabContent({ /> ) : ( > = {}; + // throw early if the defaults are invalid + EnvironmentConfigSchema.parse(defaultConfig); + + const maybeConfig: Partial> = + 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 = { ...defaultOptions, panicThreshold: 'all_errors',