From fa45d411bbd1604bdf92ae61028c3e5bf4ad924c Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 12 Jul 2025 10:02:58 -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? --- .../playground/components/Editor/EditorImpl.tsx | 11 +++++++++++ .../src/Utils/TestUtils.ts | 13 +++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 27d2fd7989..8a399aa304 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -204,6 +204,17 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { }; const parsedOptions = parseConfigPragmaForTests(pragma, { compilationMode: 'infer', + environment: { + // enabled in compiler + validateRefAccessDuringRender: true, + // enabled in linter + validateNoSetStateInRender: true, + validateNoSetStateInEffects: true, + validateNoJSXInTryStatements: true, + validateNoImpureFunctionsInRender: true, + validateStaticComponents: true, + validateNoFreezingKnownMutableFunctions: true, + }, }); const opts: PluginOptions = parsePluginOptions({ ...parsedOptions, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts index 6c2cfd5d07..e1030ff6c8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts +++ b/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> = {}; + // 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',