Skip to content

Commit

Permalink
Update on "compiler: revert #29720 to unbreak filter mode in snap"
Browse files Browse the repository at this point in the history
[ghstack-poisoned]
  • Loading branch information
josephsavona committed Jun 6, 2024
2 parents 88b9e8c + 7f7d339 commit 59bc1b7
Show file tree
Hide file tree
Showing 64 changed files with 2,566 additions and 1,002 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ module.exports = {
$ReadOnlyArray: 'readonly',
$ArrayBufferView: 'readonly',
$Shape: 'readonly',
CallSite: 'readonly',
ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be.
ReturnType: 'readonly',
AnimationFrameID: 'readonly',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ export default function BabelPluginReactCompiler(
*/
Program(prog, pass): void {
let opts = parsePluginOptions(pass.opts);
const isDev =
(typeof __DEV__ !== "undefined" && __DEV__ === true) ||
process.env["NODE_ENV"] === "development";
if (
opts.enableReanimatedCheck === true &&
pipelineUsesReanimatedPlugin(pass.file.opts.plugins)
) {
opts = injectReanimatedFlag(opts);
}
if (process.env["NODE_ENV"] === "development") {
if (isDev) {
opts = {
...opts,
environment: {
Expand Down
13 changes: 13 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
BuiltInUseInsertionEffectHookId,
BuiltInUseLayoutEffectHookId,
BuiltInUseOperatorId,
BuiltInUseReducerId,
BuiltInUseRefId,
BuiltInUseStateId,
ShapeRegistry,
Expand Down Expand Up @@ -265,6 +266,18 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
returnValueReason: ValueReason.State,
}),
],
[
"useReducer",
addHook(DEFAULT_SHAPES, {
positionalParams: [],
restParam: Effect.Freeze,
returnType: { kind: "Object", shapeId: BuiltInUseReducerId },
calleeEffect: Effect.Read,
hookKind: "useReducer",
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.ReducerState,
}),
],
[
"useRef",
addHook(DEFAULT_SHAPES, {
Expand Down
13 changes: 13 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,11 @@ export enum ValueReason {
*/
State = "state",

/**
* A value returned from `useReducer`
*/
ReducerState = "reducer-state",

/**
* Props of a component or arguments of a hook.
*/
Expand Down Expand Up @@ -1493,6 +1498,14 @@ export function isSetStateType(id: Identifier): boolean {
return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState";
}

export function isUseReducerType(id: Identifier): boolean {
return id.type.kind === "Function" && id.type.shapeId === "BuiltInUseReducer";
}

export function isDispatcherType(id: Identifier): boolean {
return id.type.kind === "Function" && id.type.shapeId === "BuiltInDispatch";
}

export function isUseEffectHookType(id: Identifier): boolean {
return (
id.type.kind === "Function" && id.type.shapeId === "BuiltInUseEffectHook"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ function addShape(
export type HookKind =
| "useContext"
| "useState"
| "useReducer"
| "useRef"
| "useEffect"
| "useLayoutEffect"
Expand Down Expand Up @@ -200,6 +201,8 @@ export const BuiltInUseEffectHookId = "BuiltInUseEffectHook";
export const BuiltInUseLayoutEffectHookId = "BuiltInUseLayoutEffectHook";
export const BuiltInUseInsertionEffectHookId = "BuiltInUseInsertionEffectHook";
export const BuiltInUseOperatorId = "BuiltInUseOperator";
export const BuiltInUseReducerId = "BuiltInUseReducer";
export const BuiltInDispatchId = "BuiltInDispatch";

// ShapeRegistry with default definitions for built-ins.
export const BUILTIN_SHAPES: ShapeRegistry = new Map();
Expand Down Expand Up @@ -387,6 +390,25 @@ addObject(BUILTIN_SHAPES, BuiltInUseStateId, [
],
]);

addObject(BUILTIN_SHAPES, BuiltInUseReducerId, [
["0", { kind: "Poly" }],
[
"1",
addFunction(
BUILTIN_SHAPES,
[],
{
positionalParams: [],
restParam: Effect.Freeze,
returnType: PRIMITIVE_TYPE,
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Primitive,
},
BuiltInDispatchId
),
],
]);

addObject(BUILTIN_SHAPES, BuiltInUseRefId, [
["current", { kind: "Object", shapeId: BuiltInRefValueId }],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Place,
computePostDominatorTree,
getHookKind,
isDispatcherType,
isSetStateType,
isUseOperator,
} from "../HIR";
Expand Down Expand Up @@ -219,7 +220,10 @@ export function inferReactivePlaces(fn: HIRFunction): void {

if (hasReactiveInput) {
for (const lvalue of eachInstructionLValue(instruction)) {
if (isSetStateType(lvalue.identifier)) {
if (
isSetStateType(lvalue.identifier) ||
isDispatcherType(lvalue.identifier)
) {
continue;
}
reactiveIdentifiers.markReactive(lvalue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2117,6 +2117,8 @@ function getWriteErrorReason(abstractValue: AbstractValue): string {
return "Mutating component props or hook arguments is not allowed. Consider using a local variable instead";
} else if (abstractValue.reason.has(ValueReason.State)) {
return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead";
} else if (abstractValue.reason.has(ValueReason.ReducerState)) {
return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
} else {
return "This mutates a variable that React considers immutable";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ReactiveFunction,
ReactiveInstruction,
ReactiveScopeBlock,
isDispatcherType,
isSetStateType,
} from "../HIR";
import { eachPatternOperand } from "../HIR/visitors";
Expand Down Expand Up @@ -56,7 +57,10 @@ class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
case "Destructure": {
if (state.has(value.value.identifier.id)) {
for (const lvalue of eachPatternOperand(value.lvalue.pattern)) {
if (isSetStateType(lvalue.identifier)) {
if (
isSetStateType(lvalue.identifier) ||
isDispatcherType(lvalue.identifier)
) {
continue;
}
state.add(lvalue.identifier.id);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

## Input

```javascript
import { useReducer } from "react";

function Foo() {
let [state, setState] = useReducer({ foo: 1 });
state.foo = 1;
return state;
}

```


## Error

```
3 | function Foo() {
4 | let [state, setState] = useReducer({ foo: 1 });
> 5 | state.foo = 1;
| ^^^^^ InvalidReact: Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead (5:5)
6 | return state;
7 | }
8 |
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { useReducer } from "react";

function Foo() {
let [state, setState] = useReducer({ foo: 1 });
state.foo = 1;
return state;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@

## Input

```javascript
import { useReducer } from "react";

function f() {
const [state, dispatch] = useReducer();

const onClick = () => {
dispatch();
};

return <div onClick={onClick} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: f,
params: [],
isComponent: true,
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { useReducer } from "react";

function f() {
const $ = _c(1);
const [state, dispatch] = useReducer();
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const onClick = () => {
dispatch();
};

t0 = <div onClick={onClick} />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: f,
params: [],
isComponent: true,
};

```
### Eval output
(kind: ok) <div></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { useReducer } from "react";

function f() {
const [state, dispatch] = useReducer();

const onClick = () => {
dispatch();
};

return <div onClick={onClick} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: f,
params: [],
isComponent: true,
};
3 changes: 2 additions & 1 deletion compiler/packages/snap/src/runner-watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,15 @@ function subscribeKeyEvents(
state: RunnerState,
onChange: (state: RunnerState) => void
) {
process.stdin.on("keypress", (str, key) => {
process.stdin.on("keypress", async (str, key) => {
if (key.name === "u") {
// u => update fixtures
state.mode.action = RunnerAction.Update;
} else if (key.name === "q") {
process.exit(0);
} else if (key.name === "f") {
state.mode.filter = !state.mode.filter;
state.filter = state.mode.filter ? await readTestFilter() : null;
state.mode.action = RunnerAction.Test;
} else {
// any other key re-runs tests
Expand Down
26 changes: 14 additions & 12 deletions fixtures/flight/server/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ if (process.env.NODE_ENV === 'development') {
res.set('Content-type', 'application/json');
let requestedFilePath = req.query.name;

let isCompiledOutput = false;
if (requestedFilePath.startsWith('file://')) {
// We assume that if it was prefixed with file:// it's referring to the compiled output
// and if it's a direct file path we assume it's source mapped back to original format.
isCompiledOutput = true;
requestedFilePath = requestedFilePath.slice(7);
}

Expand All @@ -204,11 +208,11 @@ if (process.env.NODE_ENV === 'development') {
let map;
// There are two ways to return a source map depending on what we observe in error.stack.
// A real app will have a similar choice to make for which strategy to pick.
if (!sourceMap || Error.prepareStackTrace === undefined) {
// When --enable-source-maps is enabled, the error.stack that we use to track
// stacks will have had the source map already applied so it's pointing to the
// original source. We return a blank source map that just maps everything to
// the original source in this case.
if (!sourceMap || !isCompiledOutput) {
// If a file doesn't have a source map, such as this file, then we generate a blank
// source map that just contains the original content and segments pointing to the
// original lines.
// Similarly
const sourceContent = await readFile(requestedFilePath, 'utf8');
const lines = sourceContent.split('\n').length;
map = {
Expand All @@ -222,13 +226,11 @@ if (process.env.NODE_ENV === 'development') {
sourceRoot: '',
};
} else {
// If something has overridden prepareStackTrace it is likely not getting the
// natively applied source mapping to error.stack and so the line will point to
// the compiled output similar to how a browser works.
// E.g. ironically this can happen with the source-map-support library that is
// auto-invoked by @babel/register if external source maps are generated.
// In this case we just use the source map that the native source mapping would
// have used.
// We always set prepareStackTrace before reading the stack so that we get the stack
// without source maps applied. Therefore we have to use the original source map.
// If something read .stack before we did, we might observe the line/column after
// source mapping back to the original file. We use the isCompiledOutput check above
// in that case.
map = sourceMap.payload;
}
res.write(JSON.stringify(map));
Expand Down
1 change: 0 additions & 1 deletion packages/react-devtools-core/src/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ function initialize(socket: WebSocket) {
// $FlowFixMe[incompatible-call] found when upgrading Flow
store = new Store(bridge, {
checkBridgeProtocolCompatibility: true,
supportsNativeInspection: true,
supportsTraceUpdates: true,
});

Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-extensions/src/main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ function createBridgeAndStore() {
// At this time, the timeline can only parse Chrome performance profiles.
supportsTimeline: __IS_CHROME__,
supportsTraceUpdates: true,
supportsNativeInspection: true,
});

if (!isProfiling) {
Expand Down
1 change: 0 additions & 1 deletion packages/react-devtools-fusebox/src/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store {
return new Store(bridge, {
checkBridgeProtocolCompatibility: true,
supportsTraceUpdates: true,
supportsNativeInspection: true,
...config,
});
}
Expand Down
1 change: 0 additions & 1 deletion packages/react-devtools-inline/src/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store {
checkBridgeProtocolCompatibility: true,
supportsTraceUpdates: true,
supportsTimeline: true,
supportsNativeInspection: true,
...config,
});
}
Expand Down
Loading

0 comments on commit 59bc1b7

Please sign in to comment.