Skip to content

Commit 006ae37

Browse files
josephsavonajosephsavona
authored
[compiler] Collapse CompilerError.{invariant,simpleInvariant} (facebook#35614)
`invariant()` was a pain to use - we always record a single location, but the API required passing a compiler detail. This PR replaces `invariant()` (keeping the name) with `simpleInvariant()`s signature, and updates call sites accordingly. I've noticed that agents consistently get invariant() wrong, which aligns with it being tedious to call when you're writing code by hand. The simplified API should help a bit.
1 parent a688a3d commit 006ae37

72 files changed

Lines changed: 316 additions & 2183 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,12 @@ export class CompilerError extends Error {
304304
disabledDetails: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
305305
printedMessage: string | null = null;
306306

307-
static simpleInvariant(
307+
static invariant(
308308
condition: unknown,
309309
options: {
310310
reason: CompilerDiagnosticOptions['reason'];
311311
description?: CompilerDiagnosticOptions['description'];
312+
message?: string | null;
312313
loc: SourceLocation;
313314
},
314315
): asserts condition {
@@ -322,28 +323,12 @@ export class CompilerError extends Error {
322323
}).withDetails({
323324
kind: 'error',
324325
loc: options.loc,
325-
message: options.reason,
326+
message: options.message ?? options.reason,
326327
}),
327328
);
328329
throw errors;
329330
}
330331
}
331-
static invariant(
332-
condition: unknown,
333-
options: Omit<CompilerDiagnosticOptions, 'category'>,
334-
): asserts condition {
335-
if (!condition) {
336-
const errors = new CompilerError();
337-
errors.pushDiagnostic(
338-
CompilerDiagnostic.create({
339-
reason: options.reason,
340-
description: options.description,
341-
category: ErrorCategory.Invariant,
342-
}).withDetails(...options.details),
343-
);
344-
throw errors;
345-
}
346-
}
347332

348333
static throwDiagnostic(options: CompilerDiagnosticOptions): never {
349334
const errors = new CompilerError();

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import {NodePath} from '@babel/core';
99
import * as t from '@babel/types';
1010
import {CompilerError} from '../CompilerError';
11+
import {GeneratedSource} from '../HIR';
1112
import {ProgramContext} from './Imports';
1213
import {ExternalFunction} from '..';
1314

@@ -51,26 +52,12 @@ function insertAdditionalFunctionDeclaration(
5152
CompilerError.invariant(originalFnName != null && compiled.id != null, {
5253
reason:
5354
'Expected function declarations that are referenced elsewhere to have a named identifier',
54-
description: null,
55-
details: [
56-
{
57-
kind: 'error',
58-
loc: fnPath.node.loc ?? null,
59-
message: null,
60-
},
61-
],
55+
loc: fnPath.node.loc ?? GeneratedSource,
6256
});
6357
CompilerError.invariant(originalFnParams.length === compiledParams.length, {
6458
reason:
6559
'Expected React Compiler optimized function declarations to have the same number of parameters as source',
66-
description: null,
67-
details: [
68-
{
69-
kind: 'error',
70-
loc: fnPath.node.loc ?? null,
71-
message: null,
72-
},
73-
],
60+
loc: fnPath.node.loc ?? GeneratedSource,
7461
});
7562

7663
const gatingCondition = t.identifier(
@@ -154,13 +141,7 @@ export function insertGatedFunctionDeclaration(
154141
CompilerError.invariant(compiled.type === 'FunctionDeclaration', {
155142
reason: 'Expected compiled node type to match input type',
156143
description: `Got ${compiled.type} but expected FunctionDeclaration`,
157-
details: [
158-
{
159-
kind: 'error',
160-
loc: fnPath.node.loc ?? null,
161-
message: null,
162-
},
163-
],
144+
loc: fnPath.node.loc ?? GeneratedSource,
164145
});
165146
insertAdditionalFunctionDeclaration(
166147
fnPath,

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -257,14 +257,7 @@ export function addImportsToProgram(
257257
reason:
258258
'Encountered conflicting import specifiers in generated program',
259259
description: `Conflict from import ${loweredImport.module}:(${loweredImport.imported} as ${loweredImport.name})`,
260-
details: [
261-
{
262-
kind: 'error',
263-
loc: GeneratedSource,
264-
message: null,
265-
},
266-
],
267-
suggestions: null,
260+
loc: GeneratedSource,
268261
},
269262
);
270263
CompilerError.invariant(
@@ -274,13 +267,7 @@ export function addImportsToProgram(
274267
reason:
275268
'Found inconsistent import specifier. This is an internal bug.',
276269
description: `Expected import ${moduleName}:${specifierName} but found ${loweredImport.module}:${loweredImport.imported}`,
277-
details: [
278-
{
279-
kind: 'error',
280-
loc: GeneratedSource,
281-
message: null,
282-
},
283-
],
270+
loc: GeneratedSource,
284271
},
285272
);
286273
}

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -315,13 +315,7 @@ function insertNewOutlinedFunctionNode(
315315
CompilerError.invariant(insertedFuncDecl.isFunctionDeclaration(), {
316316
reason: 'Expected inserted function declaration',
317317
description: `Got: ${insertedFuncDecl}`,
318-
details: [
319-
{
320-
kind: 'error',
321-
loc: insertedFuncDecl.node?.loc ?? null,
322-
message: null,
323-
},
324-
],
318+
loc: insertedFuncDecl.node?.loc ?? GeneratedSource,
325319
});
326320
return insertedFuncDecl;
327321
}
@@ -446,14 +440,7 @@ export function compileProgram(
446440
for (const outlined of compiled.outlined) {
447441
CompilerError.invariant(outlined.fn.outlined.length === 0, {
448442
reason: 'Unexpected nested outlined functions',
449-
description: null,
450-
details: [
451-
{
452-
kind: 'error',
453-
loc: outlined.fn.loc,
454-
message: null,
455-
},
456-
],
443+
loc: outlined.fn.loc,
457444
});
458445
const fn = insertNewOutlinedFunctionNode(
459446
program,
@@ -1451,15 +1438,7 @@ export function getReactCompilerRuntimeModule(
14511438
typeof target.runtimeModule === 'string',
14521439
{
14531440
reason: 'Expected target to already be validated',
1454-
description: null,
1455-
details: [
1456-
{
1457-
kind: 'error',
1458-
loc: null,
1459-
message: null,
1460-
},
1461-
],
1462-
suggestions: null,
1441+
loc: GeneratedSource,
14631442
},
14641443
);
14651444
return target.runtimeModule;

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,7 @@ export function suppressionsToCompilerError(
163163
): CompilerError {
164164
CompilerError.invariant(suppressionRanges.length !== 0, {
165165
reason: `Expected at least suppression comment source range`,
166-
description: null,
167-
details: [
168-
{
169-
kind: 'error',
170-
loc: GeneratedSource,
171-
message: null,
172-
},
173-
],
166+
loc: GeneratedSource,
174167
});
175168
const error = new CompilerError();
176169
for (const suppressionRange of suppressionRanges) {

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,7 @@ function validateImportSpecifier(
215215
const binding = local.scope.getBinding(local.node.name);
216216
CompilerError.invariant(binding != null, {
217217
reason: 'Expected binding to be found for import specifier',
218-
description: null,
219-
details: [
220-
{
221-
kind: 'error',
222-
loc: local.node.loc ?? null,
223-
message: null,
224-
},
225-
],
218+
loc: local.node.loc ?? GeneratedSource,
226219
});
227220
checkFn(binding.referencePaths, state);
228221
}
@@ -242,14 +235,7 @@ function validateNamespacedImport(
242235

243236
CompilerError.invariant(binding != null, {
244237
reason: 'Expected binding to be found for import specifier',
245-
description: null,
246-
details: [
247-
{
248-
kind: 'error',
249-
loc: local.node.loc ?? null,
250-
message: null,
251-
},
252-
],
238+
loc: local.node.loc ?? GeneratedSource,
253239
});
254240
const filteredReferences = new Map<
255241
CheckInvalidReferenceFn,

‎compiler/packages/babel-plugin-react-compiler/src/Flood/TypeErrors.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,7 @@ export function raiseUnificationErrors(
4646
if (errs.length === 0) {
4747
CompilerError.invariant(false, {
4848
reason: 'Should not have array of zero errors',
49-
description: null,
50-
details: [
51-
{
52-
kind: 'error',
53-
loc,
54-
message: null,
55-
},
56-
],
49+
loc,
5750
});
5851
} else if (errs.length === 1) {
5952
CompilerError.throwInvalidJS({

‎compiler/packages/babel-plugin-react-compiler/src/Flood/Types.ts‎

Lines changed: 9 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,7 @@ export type LinearId = number & {
151151
export function makeLinearId(id: number): LinearId {
152152
CompilerError.invariant(id >= 0 && Number.isInteger(id), {
153153
reason: 'Expected LinearId id to be a non-negative integer',
154-
description: null,
155-
details: [
156-
{
157-
kind: 'error',
158-
loc: null,
159-
message: null,
160-
},
161-
],
162-
suggestions: null,
154+
loc: GeneratedSource,
163155
});
164156
return id as LinearId;
165157
}
@@ -172,15 +164,7 @@ export type TypeParameterId = number & {
172164
export function makeTypeParameterId(id: number): TypeParameterId {
173165
CompilerError.invariant(id >= 0 && Number.isInteger(id), {
174166
reason: 'Expected TypeParameterId to be a non-negative integer',
175-
description: null,
176-
details: [
177-
{
178-
kind: 'error',
179-
loc: null,
180-
message: null,
181-
},
182-
],
183-
suggestions: null,
167+
loc: GeneratedSource,
184168
});
185169
return id as TypeParameterId;
186170
}
@@ -202,15 +186,7 @@ export type VariableId = number & {
202186
export function makeVariableId(id: number): VariableId {
203187
CompilerError.invariant(id >= 0 && Number.isInteger(id), {
204188
reason: 'Expected VariableId id to be a non-negative integer',
205-
description: null,
206-
details: [
207-
{
208-
kind: 'error',
209-
loc: null,
210-
message: null,
211-
},
212-
],
213-
suggestions: null,
189+
loc: GeneratedSource,
214190
});
215191
return id as VariableId;
216192
}
@@ -417,14 +393,7 @@ function convertFlowType(flowType: FlowType, loc: string): ResolvedType {
417393
} else {
418394
CompilerError.invariant(false, {
419395
reason: `Unsupported property kind ${prop.kind}`,
420-
description: null,
421-
details: [
422-
{
423-
kind: 'error',
424-
loc: GeneratedSource,
425-
message: null,
426-
},
427-
],
396+
loc: GeneratedSource,
428397
});
429398
}
430399
}
@@ -493,14 +462,7 @@ function convertFlowType(flowType: FlowType, loc: string): ResolvedType {
493462
} else {
494463
CompilerError.invariant(false, {
495464
reason: `Unsupported property kind ${prop.kind}`,
496-
description: null,
497-
details: [
498-
{
499-
kind: 'error',
500-
loc: GeneratedSource,
501-
message: null,
502-
},
503-
],
465+
loc: GeneratedSource,
504466
});
505467
}
506468
}
@@ -519,14 +481,7 @@ function convertFlowType(flowType: FlowType, loc: string): ResolvedType {
519481
} else {
520482
CompilerError.invariant(false, {
521483
reason: `Unsupported property kind ${prop.kind}`,
522-
description: null,
523-
details: [
524-
{
525-
kind: 'error',
526-
loc: GeneratedSource,
527-
message: null,
528-
},
529-
],
484+
loc: GeneratedSource,
530485
});
531486
}
532487
}
@@ -539,14 +494,7 @@ function convertFlowType(flowType: FlowType, loc: string): ResolvedType {
539494
}
540495
CompilerError.invariant(false, {
541496
reason: `Unsupported class instance type ${flowType.def.type.kind}`,
542-
description: null,
543-
details: [
544-
{
545-
kind: 'error',
546-
loc: GeneratedSource,
547-
message: null,
548-
},
549-
],
497+
loc: GeneratedSource,
550498
});
551499
}
552500
case 'Fun':
@@ -605,14 +553,7 @@ function convertFlowType(flowType: FlowType, loc: string): ResolvedType {
605553
} else {
606554
CompilerError.invariant(false, {
607555
reason: `Unsupported component props type ${propsType.type.kind}`,
608-
description: null,
609-
details: [
610-
{
611-
kind: 'error',
612-
loc: GeneratedSource,
613-
message: null,
614-
},
615-
],
556+
loc: GeneratedSource,
616557
});
617558
}
618559

@@ -765,14 +706,7 @@ export class FlowTypeEnv implements ITypeEnv {
765706
// TODO: use flow-js only for web environments (e.g. playground)
766707
CompilerError.invariant(env.config.flowTypeProvider != null, {
767708
reason: 'Expected flowDumpTypes to be defined in environment config',
768-
description: null,
769-
details: [
770-
{
771-
kind: 'error',
772-
loc: GeneratedSource,
773-
message: null,
774-
},
775-
],
709+
loc: GeneratedSource,
776710
});
777711
let stdout: any;
778712
if (source === lastFlowSource) {

0 commit comments

Comments
 (0)