Skip to content

Commit bcea869

Browse files
authored
[compiler][rfc] Enable more validations in playground. (facebook#33777)
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? --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33777). * facebook#33981 * __->__ facebook#33777
1 parent 2ae8b3d commit bcea869

File tree

3 files changed

+99
-21
lines changed

3 files changed

+99
-21
lines changed

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

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ const COMMON_HOOKS: Array<[string, Hook]> = [
142142
],
143143
];
144144

145-
function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
145+
function compile(
146+
source: string,
147+
mode: 'compiler' | 'linter',
148+
): [CompilerOutput, 'flow' | 'typescript'] {
146149
const results = new Map<string, Array<PrintedCompilerPipelineValue>>();
147150
const error = new CompilerError();
148151
const otherErrors: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
@@ -204,6 +207,22 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
204207
};
205208
const parsedOptions = parseConfigPragmaForTests(pragma, {
206209
compilationMode: 'infer',
210+
environment:
211+
mode === 'linter'
212+
? {
213+
// enabled in compiler
214+
validateRefAccessDuringRender: false,
215+
// enabled in linter
216+
validateNoSetStateInRender: true,
217+
validateNoSetStateInEffects: true,
218+
validateNoJSXInTryStatements: true,
219+
validateNoImpureFunctionsInRender: true,
220+
validateStaticComponents: true,
221+
validateNoFreezingKnownMutableFunctions: true,
222+
}
223+
: {
224+
/* use defaults for compiler mode */
225+
},
207226
});
208227
const opts: PluginOptions = parsePluginOptions({
209228
...parsedOptions,
@@ -249,9 +268,12 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
249268
otherErrors.forEach(e => error.details.push(e));
250269
}
251270
if (error.hasErrors()) {
252-
return [{kind: 'err', results, error: error}, language];
271+
return [{kind: 'err', results, error}, language];
253272
}
254-
return [{kind: 'ok', results, transformOutput}, language];
273+
return [
274+
{kind: 'ok', results, transformOutput, errors: error.details},
275+
language,
276+
];
255277
}
256278

257279
export default function Editor(): JSX.Element {
@@ -260,7 +282,11 @@ export default function Editor(): JSX.Element {
260282
const dispatchStore = useStoreDispatch();
261283
const {enqueueSnackbar} = useSnackbar();
262284
const [compilerOutput, language] = useMemo(
263-
() => compile(deferredStore.source),
285+
() => compile(deferredStore.source, 'compiler'),
286+
[deferredStore.source],
287+
);
288+
const [linterOutput] = useMemo(
289+
() => compile(deferredStore.source, 'linter'),
264290
[deferredStore.source],
265291
);
266292

@@ -286,19 +312,26 @@ export default function Editor(): JSX.Element {
286312
});
287313
});
288314

315+
let mergedOutput: CompilerOutput;
316+
let errors: Array<CompilerErrorDetail | CompilerDiagnostic>;
317+
if (compilerOutput.kind === 'ok') {
318+
errors = linterOutput.kind === 'ok' ? [] : linterOutput.error.details;
319+
mergedOutput = {
320+
...compilerOutput,
321+
errors,
322+
};
323+
} else {
324+
mergedOutput = compilerOutput;
325+
errors = compilerOutput.error.details;
326+
}
289327
return (
290328
<>
291329
<div className="relative flex basis top-14">
292330
<div className={clsx('relative sm:basis-1/4')}>
293-
<Input
294-
language={language}
295-
errors={
296-
compilerOutput.kind === 'err' ? compilerOutput.error.details : []
297-
}
298-
/>
331+
<Input language={language} errors={errors} />
299332
</div>
300333
<div className={clsx('flex sm:flex flex-wrap')}>
301-
<Output store={deferredStore} compilerOutput={compilerOutput} />
334+
<Output store={deferredStore} compilerOutput={mergedOutput} />
302335
</div>
303336
</div>
304337
</>

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

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import {
1111
InformationCircleIcon,
1212
} from '@heroicons/react/outline';
1313
import MonacoEditor, {DiffEditor} from '@monaco-editor/react';
14-
import {type CompilerError} from 'babel-plugin-react-compiler';
14+
import {
15+
CompilerErrorDetail,
16+
CompilerDiagnostic,
17+
type CompilerError,
18+
} from 'babel-plugin-react-compiler';
1519
import parserBabel from 'prettier/plugins/babel';
1620
import * as prettierPluginEstree from 'prettier/plugins/estree';
1721
import * as prettier from 'prettier/standalone';
@@ -44,6 +48,7 @@ export type CompilerOutput =
4448
kind: 'ok';
4549
transformOutput: CompilerTransformOutput;
4650
results: Map<string, Array<PrintedCompilerPipelineValue>>;
51+
errors: Array<CompilerErrorDetail | CompilerDiagnostic>;
4752
}
4853
| {
4954
kind: 'err';
@@ -123,10 +128,36 @@ async function tabify(
123128
parser: transformOutput.language === 'flow' ? 'babel-flow' : 'babel-ts',
124129
plugins: [parserBabel, prettierPluginEstree],
125130
});
131+
132+
let output: string;
133+
let language: string;
134+
if (compilerOutput.errors.length === 0) {
135+
output = code;
136+
language = 'javascript';
137+
} else {
138+
language = 'markdown';
139+
output = `
140+
# Output
141+
142+
React Compiler compiled this function sucessfully, but there are lint errors that indicate potential issues with the original code.
143+
144+
## ${compilerOutput.errors.length} Lint Errors
145+
146+
${compilerOutput.errors.map(e => e.printErrorMessage(source, {eslint: false})).join('\n\n')}
147+
148+
## Output
149+
150+
\`\`\`js
151+
${code}
152+
\`\`\`
153+
`.trim();
154+
}
155+
126156
reorderedTabs.set(
127-
'JS',
157+
'Output',
128158
<TextTabContent
129-
output={code}
159+
output={output}
160+
language={language}
130161
diff={null}
131162
showInfoPanel={false}></TextTabContent>,
132163
);
@@ -147,9 +178,10 @@ async function tabify(
147178
eslint: false,
148179
});
149180
reorderedTabs.set(
150-
'Errors',
181+
'Output',
151182
<TextTabContent
152183
output={errors}
184+
language="plaintext"
153185
diff={null}
154186
showInfoPanel={false}></TextTabContent>,
155187
);
@@ -173,7 +205,9 @@ function getSourceMapUrl(code: string, map: string): string | null {
173205
}
174206

175207
function Output({store, compilerOutput}: Props): JSX.Element {
176-
const [tabsOpen, setTabsOpen] = useState<Set<string>>(() => new Set(['JS']));
208+
const [tabsOpen, setTabsOpen] = useState<Set<string>>(
209+
() => new Set(['Output']),
210+
);
177211
const [tabs, setTabs] = useState<Map<string, React.ReactNode>>(
178212
() => new Map(),
179213
);
@@ -187,7 +221,7 @@ function Output({store, compilerOutput}: Props): JSX.Element {
187221
);
188222
if (compilerOutput.kind !== previousOutputKind) {
189223
setPreviousOutputKind(compilerOutput.kind);
190-
setTabsOpen(new Set([compilerOutput.kind === 'ok' ? 'JS' : 'Errors']));
224+
setTabsOpen(new Set(['Output']));
191225
}
192226

193227
useEffect(() => {
@@ -196,7 +230,7 @@ function Output({store, compilerOutput}: Props): JSX.Element {
196230
});
197231
}, [store.source, compilerOutput]);
198232

199-
const changedPasses: Set<string> = new Set(['JS', 'HIR']); // Initial and final passes should always be bold
233+
const changedPasses: Set<string> = new Set(['Output', 'HIR']); // Initial and final passes should always be bold
200234
let lastResult: string = '';
201235
for (const [passName, results] of compilerOutput.results) {
202236
for (const result of results) {
@@ -228,10 +262,12 @@ function TextTabContent({
228262
output,
229263
diff,
230264
showInfoPanel,
265+
language,
231266
}: {
232267
output: string;
233268
diff: string | null;
234269
showInfoPanel: boolean;
270+
language: string;
235271
}): JSX.Element {
236272
const [diffMode, setDiffMode] = useState(false);
237273
return (
@@ -282,7 +318,7 @@ function TextTabContent({
282318
/>
283319
) : (
284320
<MonacoEditor
285-
defaultLanguage="javascript"
321+
language={language ?? 'javascript'}
286322
value={output}
287323
options={{
288324
...monacoOptions,

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,13 @@ function* splitPragma(
113113
*/
114114
function parseConfigPragmaEnvironmentForTest(
115115
pragma: string,
116+
defaultConfig: PartialEnvironmentConfig,
116117
): EnvironmentConfig {
117-
const maybeConfig: Partial<Record<keyof EnvironmentConfig, unknown>> = {};
118+
// throw early if the defaults are invalid
119+
EnvironmentConfigSchema.parse(defaultConfig);
120+
121+
const maybeConfig: Partial<Record<keyof EnvironmentConfig, unknown>> =
122+
defaultConfig;
118123

119124
for (const {key, value: val} of splitPragma(pragma)) {
120125
if (!hasOwnProperty(EnvironmentConfigSchema.shape, key)) {
@@ -174,9 +179,13 @@ export function parseConfigPragmaForTests(
174179
pragma: string,
175180
defaults: {
176181
compilationMode: CompilationMode;
182+
environment?: PartialEnvironmentConfig;
177183
},
178184
): PluginOptions {
179-
const environment = parseConfigPragmaEnvironmentForTest(pragma);
185+
const environment = parseConfigPragmaEnvironmentForTest(
186+
pragma,
187+
defaults.environment ?? {},
188+
);
180189
const options: Record<keyof PluginOptions, unknown> = {
181190
...defaultOptions,
182191
panicThreshold: 'all_errors',

0 commit comments

Comments
 (0)