Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default function BabelPluginReactCompiler(
pass.filename ?? null,
opts.logger,
opts.environment,
result?.retryErrors ?? [],
result,
);
if (ENABLE_REACT_COMPILER_TIMINGS === true) {
performance.mark(`${filename}:end`, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
findProgramSuppressions,
suppressionsToCompilerError,
} from './Suppression';
import {GeneratedSource} from '../HIR';

export type CompilerPass = {
opts: PluginOptions;
Expand Down Expand Up @@ -267,8 +268,9 @@ function isFilePartOfSources(
return false;
}

type CompileProgramResult = {
export type CompileProgramResult = {
retryErrors: Array<{fn: BabelFn; error: CompilerError}>;
inferredEffectLocations: Set<t.SourceLocation>;
};
/**
* `compileProgram` is directly invoked by the react-compiler babel plugin, so
Expand Down Expand Up @@ -369,6 +371,7 @@ export function compileProgram(
},
);
const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = [];
const inferredEffectLocations = new Set<t.SourceLocation>();
const processFn = (
fn: BabelFn,
fnType: ReactFunctionType,
Expand Down Expand Up @@ -509,6 +512,14 @@ export function compileProgram(
if (!pass.opts.noEmit) {
return compileResult.compiledFn;
}
/**
* inferEffectDependencies + noEmit is currently only used for linting. In
* this mode, add source locations for where the compiler *can* infer effect
* dependencies.
*/
for (const loc of compileResult.compiledFn.inferredEffectLocations) {
if (loc !== GeneratedSource) inferredEffectLocations.add(loc);
}
return null;
};

Expand Down Expand Up @@ -587,7 +598,7 @@ export function compileProgram(
if (compiledFns.length > 0) {
addImportsToProgram(program, programContext);
}
return {retryErrors};
return {retryErrors, inferredEffectLocations};
}

function shouldSkipCompilation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import {getOrInsertWith} from '../Utils/utils';
import {Environment} from '../HIR';
import {DEFAULT_EXPORT} from '../HIR/Environment';
import {CompileProgramResult} from './Program';

function throwInvalidReact(
options: Omit<CompilerErrorDetailOptions, 'severity'>,
Expand All @@ -36,12 +37,16 @@ function assertValidEffectImportReference(
const parent = path.parentPath;
if (parent != null && parent.isCallExpression()) {
const args = parent.get('arguments');
const maybeCalleeLoc = path.node.loc;
const hasInferredEffect =
maybeCalleeLoc != null &&
context.inferredEffectLocations.has(maybeCalleeLoc);
/**
* Only error on untransformed references of the form `useMyEffect(...)`
* or `moduleNamespace.useMyEffect(...)`, with matching argument counts.
* TODO: do we also want a mode to also hard error on non-call references?
*/
if (args.length === numArgs) {
if (args.length === numArgs && !hasInferredEffect) {
const maybeErrorDiagnostic = matchCompilerDiagnostic(
path,
context.transformErrors,
Expand Down Expand Up @@ -97,7 +102,7 @@ export default function validateNoUntransformedReferences(
filename: string | null,
logger: Logger | null,
env: EnvironmentConfig,
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>,
compileResult: CompileProgramResult | null,
): void {
const moduleLoadChecks = new Map<
string,
Expand Down Expand Up @@ -126,7 +131,7 @@ export default function validateNoUntransformedReferences(
}
}
if (moduleLoadChecks.size > 0) {
transformProgram(path, moduleLoadChecks, filename, logger, transformErrors);
transformProgram(path, moduleLoadChecks, filename, logger, compileResult);
}
}

Expand All @@ -136,6 +141,7 @@ type TraversalState = {
logger: Logger | null;
filename: string | null;
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>;
inferredEffectLocations: Set<t.SourceLocation>;
};
type CheckInvalidReferenceFn = (
paths: Array<NodePath<t.Node>>,
Expand Down Expand Up @@ -223,14 +229,16 @@ function transformProgram(
moduleLoadChecks: Map<string, Map<string, CheckInvalidReferenceFn>>,
filename: string | null,
logger: Logger | null,
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>,
compileResult: CompileProgramResult | null,
): void {
const traversalState: TraversalState = {
shouldInvalidateScopes: true,
program: path,
filename,
logger,
transformErrors,
transformErrors: compileResult?.retryErrors ?? [],
inferredEffectLocations:
compileResult?.inferredEffectLocations ?? new Set(),
};
path.traverse({
ImportDeclaration(path: NodePath<t.ImportDeclaration>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {fromZodError} from 'zod-validation-error';
import {CompilerError} from '../CompilerError';
import {
CompilationMode,
defaultOptions,
Logger,
PanicThresholdOptions,
parsePluginOptions,
Expand Down Expand Up @@ -779,6 +780,7 @@ export function parseConfigPragmaForTests(
const environment = parseConfigPragmaEnvironmentForTest(pragma);
let compilationMode: CompilationMode = defaults.compilationMode;
let panicThreshold: PanicThresholdOptions = 'all_errors';
let noEmit: boolean = defaultOptions.noEmit;
for (const token of pragma.split(' ')) {
if (!token.startsWith('@')) {
continue;
Expand All @@ -804,12 +806,17 @@ export function parseConfigPragmaForTests(
panicThreshold = 'none';
break;
}
case '@noEmit': {
noEmit = true;
break;
}
}
}
return parsePluginOptions({
environment,
compilationMode,
panicThreshold,
noEmit,
});
}

Expand Down Expand Up @@ -852,6 +859,7 @@ export class Environment {
programContext: ProgramContext;
hasFireRewrite: boolean;
hasInferredEffect: boolean;
inferredEffectLocations: Set<SourceLocation> = new Set();

#contextIdentifiers: Set<t.Identifier>;
#hoistedIdentifiers: Set<t.Identifier>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
// Step 2: push the inferred deps array as an argument of the useEffect
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
fn.env.inferredEffectLocations.add(callee.loc);
} else if (loadGlobals.has(value.args[0].identifier.id)) {
// Global functions have no reactive dependencies, so we can insert an empty array
newInstructions.push({
Expand All @@ -227,6 +228,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
});
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
fn.env.inferredEffectLocations.add(callee.loc);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export type CodegenFunction = {
* This is true if the compiler has compiled inferred effect dependencies
*/
hasInferredEffect: boolean;
inferredEffectLocations: Set<SourceLocation>;

/**
* This is true if the compiler has compiled a fire to a useFire call
Expand Down Expand Up @@ -389,6 +390,7 @@ function codegenReactiveFunction(
outlined: [],
hasFireRewrite: fn.env.hasFireRewrite,
hasInferredEffect: fn.env.hasInferredEffect,
inferredEffectLocations: fn.env.inferredEffectLocations,
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@

## Input

```javascript
// @inferEffectDependencies @noEmit
import {print} from 'shared-runtime';
import useEffectWrapper from 'useEffectWrapper';

function ReactiveVariable({propVal}) {
const arr = [propVal];
useEffectWrapper(() => print(arr));
}

```

## Code

```javascript
// @inferEffectDependencies @noEmit
import { print } from "shared-runtime";
import useEffectWrapper from "useEffectWrapper";

function ReactiveVariable({ propVal }) {
const arr = [propVal];
useEffectWrapper(() => print(arr));
}

```

### Eval output
(kind: exception) Fixture not implemented
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @inferEffectDependencies @noEmit
import {print} from 'shared-runtime';
import useEffectWrapper from 'useEffectWrapper';

function ReactiveVariable({propVal}) {
const arr = [propVal];
useEffectWrapper(() => print(arr));
}
1 change: 0 additions & 1 deletion compiler/packages/snap/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ function makePluginOptions(
},
logger,
gating,
noEmit: false,
eslintSuppressionRules,
flowSuppressions,
ignoreUseNoForget,
Expand Down
Loading