Skip to content

Commit

Permalink
Update base for Update on "[compiler] Validate against JSX in try sta…
Browse files Browse the repository at this point in the history
…tements"

Per comments on the new validation pass, this disallows creating JSX (expression/fragment) within a try statement. Developers sometimes use this pattern thinking that they can catch errors during the rendering of the element, without realizing that rendering is lazy. The validation allows us to teach developers about the error boundary pattern.

[ghstack-poisoned]
  • Loading branch information
josephsavona committed Aug 16, 2024
2 parents 5edbe29 + a58276c commit 7e29079
Show file tree
Hide file tree
Showing 28 changed files with 1,157 additions and 234 deletions.
2 changes: 1 addition & 1 deletion ReactVersions.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const stablePackages = {
// These packages do not exist in the @canary or @latest channel, only
// @experimental. We don't use semver, just the commit sha, so this is just a
// list of package names instead of a map.
const experimentalPackages = [];
const experimentalPackages = ['react-markup'];

module.exports = {
ReactVersion,
Expand Down
63 changes: 34 additions & 29 deletions compiler/apps/playground/components/Editor/EditorImpl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ function parseFunctions(
source: string,
language: 'flow' | 'typescript',
): Array<
NodePath<
t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
>
| NodePath<t.FunctionDeclaration>
| NodePath<t.ArrowFunctionExpression>
| NodePath<t.FunctionExpression>
> {
const items: Array<
NodePath<
t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
>
| NodePath<t.FunctionDeclaration>
| NodePath<t.ArrowFunctionExpression>
| NodePath<t.FunctionExpression>
> = [];
try {
const ast = parseInput(source, language);
Expand Down Expand Up @@ -155,22 +155,33 @@ function isHookName(s: string): boolean {
return /^use[A-Z0-9]/.test(s);
}

function getReactFunctionType(
id: NodePath<t.Identifier | null | undefined>,
): ReactFunctionType {
if (id && id.node && id.isIdentifier()) {
if (isHookName(id.node.name)) {
function getReactFunctionType(id: t.Identifier | null): ReactFunctionType {
if (id != null) {
if (isHookName(id.name)) {
return 'Hook';
}

const isPascalCaseNameSpace = /^[A-Z].*/;
if (isPascalCaseNameSpace.test(id.node.name)) {
if (isPascalCaseNameSpace.test(id.name)) {
return 'Component';
}
}
return 'Other';
}

function getFunctionIdentifier(
fn:
| NodePath<t.FunctionDeclaration>
| NodePath<t.ArrowFunctionExpression>
| NodePath<t.FunctionExpression>,
): t.Identifier | null {
if (fn.isArrowFunctionExpression()) {
return null;
}
const id = fn.get('id');
return Array.isArray(id) === false && id.isIdentifier() ? id.node : null;
}

function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
const results = new Map<string, PrintedCompilerPipelineValue[]>();
const error = new CompilerError();
Expand All @@ -188,27 +199,21 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
} else {
language = 'typescript';
}
let count = 0;
const withIdentifier = (id: t.Identifier | null): t.Identifier => {
if (id != null && id.name != null) {
return id;
} else {
return t.identifier(`anonymous_${count++}`);
}
};
try {
// Extract the first line to quickly check for custom test directives
const pragma = source.substring(0, source.indexOf('\n'));
const config = parseConfigPragma(pragma);

for (const fn of parseFunctions(source, language)) {
if (!fn.isFunctionDeclaration()) {
error.pushErrorDetail(
new CompilerErrorDetail({
reason: `Unexpected function type ${fn.node.type}`,
description:
'Playground only supports parsing function declarations',
severity: ErrorSeverity.Todo,
loc: fn.node.loc ?? null,
suggestions: null,
}),
);
continue;
}

const id = fn.get('id');
const id = withIdentifier(getFunctionIdentifier(fn));
for (const result of run(
fn,
{
Expand All @@ -221,7 +226,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
null,
null,
)) {
const fnName = fn.node.id?.name ?? null;
const fnName = id.name;
switch (result.kind) {
case 'ast': {
upsert({
Expand All @@ -230,7 +235,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
name: result.name,
value: {
type: 'FunctionDeclaration',
id: result.value.id,
id,
async: result.value.async,
generator: result.value.generator,
body: result.value.body,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@

import * as t from '@babel/types';
import {z} from 'zod';
import {CompilerErrorDetailOptions} from '../CompilerError';
import {ExternalFunction, PartialEnvironmentConfig} from '../HIR/Environment';
import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError';
import {
EnvironmentConfig,
ExternalFunction,
parseEnvironmentConfig,
} from '../HIR/Environment';
import {hasOwnProperty} from '../Utils/utils';

const PanicThresholdOptionsSchema = z.enum([
Expand All @@ -32,7 +36,7 @@ const PanicThresholdOptionsSchema = z.enum([
export type PanicThresholdOptions = z.infer<typeof PanicThresholdOptionsSchema>;

export type PluginOptions = {
environment: PartialEnvironmentConfig | null;
environment: EnvironmentConfig;

logger: Logger | null;

Expand Down Expand Up @@ -165,6 +169,12 @@ export type LoggerEvent =
fnLoc: t.SourceLocation | null;
detail: Omit<Omit<CompilerErrorDetailOptions, 'severity'>, 'suggestions'>;
}
| {
kind: 'CompileSkip';
fnLoc: t.SourceLocation | null;
reason: string;
loc: t.SourceLocation | null;
}
| {
kind: 'CompileSuccess';
fnLoc: t.SourceLocation | null;
Expand All @@ -188,7 +198,7 @@ export type Logger = {
export const defaultOptions: PluginOptions = {
compilationMode: 'infer',
panicThreshold: 'none',
environment: {},
environment: parseEnvironmentConfig({}).unwrap(),
logger: null,
gating: null,
noEmit: false,
Expand All @@ -212,7 +222,19 @@ export function parsePluginOptions(obj: unknown): PluginOptions {
// normalize string configs to be case insensitive
value = value.toLowerCase();
}
if (isCompilerFlag(key)) {
if (key === 'environment') {
const environmentResult = parseEnvironmentConfig(value);
if (environmentResult.isErr()) {
CompilerError.throwInvalidConfig({
reason:
'Error in validating environment config. This is an advanced setting and not meant to be used directly',
description: environmentResult.unwrapErr().toString(),
suggestions: null,
loc: null,
});
}
parsedOptions[key] = environmentResult.unwrap();
} else if (isCompilerFlag(key)) {
parsedOptions[key] = value;
}
}
Expand Down
Loading

0 comments on commit 7e29079

Please sign in to comment.