@samitouri / QOS-React-1 / commits / 683a197a89

Manually revert #2127 (allow mutating context in callbacks)

#2127 introduced a special type for the result of `useContext()` that was sort of ref-like. The intent was to allow code like this: ``` function Foo() { const cx = useContext(...); function onEvent() { cx.foo = true; }; return <Bar onEvent={onEvent} />; } ``` However, that code actually is allowed by the compiler by default. It's only a bailout when `@validateFrozenLambdas` is enabled. The "fix" in #2127 therefore wasn't strictly necessary to unblock rollout, and it's also flawed in a few ways: * First, `useContext(FooContext)` should have equivalent behavior to a custom hooks which does the same thing, ie `function useFooContext() { return useContext(FooContext) }`. Specializing the type of useContext makes the behavior different. * Second, it meant that even readonly accesses of the context inside a callback marked the function as capturing, which in turn prevented those callbacks from being memoized. So i'm reverting this and we'll have to think a bit more about this case.

Joe Savona committed Oct 9, 2023 at 15:21 UTC 683a197a899b2914c80478b389132a8081e4eb24

17 files changed +429 -98

compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts

+1 -2

@@ -9,7 +9,6 @@ import { Effect, ValueKind } from "./HIR";

9 import {

10 BUILTIN_SHAPES,

11 BuiltInArrayId,

12 - BuiltInContextId,

12 BuiltInUseRefId,

13 BuiltInUseStateId,

14 ShapeRegistry,

@@ -241,7 +240,7 @@ const BUILTIN_HOOKS: Array<[string, BuiltInType]> = [

240 addHook(DEFAULT_SHAPES, [], {

241 positionalParams: [],

242 restParam: Effect.Read,

244 - returnType: { kind: "Object", shapeId: BuiltInContextId },

243 + returnType: { kind: "Poly" },

244 calleeEffect: Effect.Read,

245 hookKind: "useContext",

246 returnValueKind: ValueKind.Mutable,

compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts

-4

@@ -1111,10 +1111,6 @@ export function isSetStateType(id: Identifier): boolean {

1111 return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState";

1112 }

1113

1114 -export function isContextType(id: Identifier): boolean {

1115 - return id.type.kind === "Object" && id.type.shapeId === "BuiltInContext";

1116 -}

1117 -

1114 export function getHookKind(env: Environment, id: Identifier): HookKind | null {

1115 const idType = id.type;

1116 if (idType.kind === "Function") {

compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts

-5

@@ -171,7 +171,6 @@ export const BuiltInUseStateId = "BuiltInUseState";

171 export const BuiltInSetStateId = "BuiltInSetState";

172 export const BuiltInUseRefId = "BuiltInUseRefId";

173 export const BuiltInRefValueId = "BuiltInRefValue";

174 -export const BuiltInContextId = "BuiltInContext";

174 export const BuiltInMixedReadonlyId = "BuiltInMixedReadonly";

175

176 /**

@@ -295,10 +294,6 @@ addObject(BUILTIN_SHAPES, BuiltInUseRefId, [

294 ["current", { kind: "Object", shapeId: BuiltInRefValueId }],

295 ]);

296

298 -addObject(BUILTIN_SHAPES, BuiltInContextId, [

299 - ["*", { kind: "Object", shapeId: BuiltInContextId }],

300 -]);

301 -

297 addObject(BUILTIN_SHAPES, BuiltInRefValueId, []);

298

299 addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [

compiler/packages/babel-plugin-react-forget/src/Inference/AnalyseFunctions.ts

+1 -3

@@ -10,7 +10,6 @@ import {

10 Effect,

11 HIRFunction,

12 Identifier,

13 - isContextType,

13 isRefValueType,

14 isSetStateType,

15 isUseRefType,

@@ -143,8 +142,7 @@ function infer(

142 if (

143 isUseRefType(dep.identifier) ||

144 isRefValueType(dep.identifier) ||

146 - isSetStateType(dep.identifier) ||

147 - isContextType(dep.identifier)

145 + isSetStateType(dep.identifier)

146 ) {

147 // TODO: this is a hack to ensure we treat functions which reference refs

148 // as having a capture and therefore being considered mutable. this ensures

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/maybe-mutate-object-in-callback.expect.md new

+77

@@ -0,0 +1,77 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +const { mutate } = require("shared-runtime");

6 +

7 +function Component(props) {

8 + const object = {};

9 + // We optimistically assume function calls within callbacks don't mutate (unless the function

10 + // is known to be called during render), so this should get memoized

11 + const onClick = () => {

12 + mutate(object);

13 + };

14 + return <Foo callback={onClick}>{props.children}</Foo>;

15 +}

16 +

17 +function Foo({ children }) {

18 + return children;

19 +}

20 +

21 +export const FIXTURE_ENTRYPOINT = {

22 + fn: Component,

23 + params: [{ children: <div>Hello</div> }],

24 +};

25 +

26 +```

27 +

28 +## Code

29 +

30 +```javascript

31 +import { unstable_useMemoCache as useMemoCache } from "react";

32 +const { mutate } = require("shared-runtime");

33 +

34 +function Component(props) {

35 + const $ = useMemoCache(4);

36 + let t0;

37 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {

38 + t0 = {};

39 + $[0] = t0;

40 + } else {

41 + t0 = $[0];

42 + }

43 + const object = t0;

44 + let t1;

45 + if ($[1] === Symbol.for("react.memo_cache_sentinel")) {

46 + t1 = () => {

47 + mutate(object);

48 + };

49 + $[1] = t1;

50 + } else {

51 + t1 = $[1];

52 + }

53 + const onClick = t1;

54 + const c_2 = $[2] !== props.children;

55 + let t2;

56 + if (c_2) {

57 + t2 = <Foo callback={onClick}>{props.children}</Foo>;

58 + $[2] = props.children;

59 + $[3] = t2;

60 + } else {

61 + t2 = $[3];

62 + }

63 + return t2;

64 +}

65 +

66 +function Foo(t5) {

67 + const { children } = t5;

68 + return children;

69 +}

70 +

71 +export const FIXTURE_ENTRYPOINT = {

72 + fn: Component,

73 + params: [{ children: <div>Hello</div> }],

74 +};

75 +

76 +```

77 +

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/maybe-mutate-object-in-callback.js new

+20

@@ -0,0 +1,20 @@

1 +const { mutate } = require("shared-runtime");

2 +

3 +function Component(props) {

4 + const object = {};

5 + // We optimistically assume function calls within callbacks don't mutate (unless the function

6 + // is known to be called during render), so this should get memoized

7 + const onClick = () => {

8 + mutate(object);

9 + };

10 + return <Foo callback={onClick}>{props.children}</Foo>;

11 +}

12 +

13 +function Foo({ children }) {

14 + return children;

15 +}

16 +

17 +export const FIXTURE_ENTRYPOINT = {

18 + fn: Component,

19 + params: [{ children: <div>Hello</div> }],

20 +};

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/react-namespace.expect.md

+18 -27

@@ -30,45 +30,36 @@ import { unstable_useMemoCache as useMemoCache } from "react";

30 const FooContext = React.createContext({ current: null });

32 function Component(props) {

33 - const $ = useMemoCache(7);

33 + const $ = useMemoCache(5);

34 const foo = React.useContext(FooContext);

35 const ref = React.useRef();

36 const [x, setX] = React.useState(false);

37 - const c_0 = $[0] !== foo.current;

37 + const onClick = () => {

38 + setX(true);

39 + ref.current = true;

40 + foo.current = true;

41 + };

42 + const c_0 = $[0] !== props.children;

43 let t0;

44 if (c_0) {

40 - t0 = () => {

41 - setX(true);

42 - ref.current = true;

43 - foo.current = true;

44 - };

45 - $[0] = foo.current;

45 + t0 = React.cloneElement(props.children);

46 + $[0] = props.children;

47 $[1] = t0;

48 } else {

49 t0 = $[1];

50 }

50 - const onClick = t0;

51 - const c_2 = $[2] !== props.children;

51 + const c_2 = $[2] !== onClick;

52 + const c_3 = $[3] !== t0;

53 let t1;

53 - if (c_2) {

54 - t1 = React.cloneElement(props.children);

55 - $[2] = props.children;

56 - $[3] = t1;

57 - } else {

58 - t1 = $[3];

59 - }

60 - const c_4 = $[4] !== onClick;

61 - const c_5 = $[5] !== t1;

62 - let t2;

63 - if (c_4 || c_5) {

64 - t2 = <div onClick={onClick}>{t1}</div>;

65 - $[4] = onClick;

66 - $[5] = t1;

67 - $[6] = t2;

54 + if (c_2 || c_3) {

55 + t1 = <div onClick={onClick}>{t0}</div>;

56 + $[2] = onClick;

57 + $[3] = t0;

58 + $[4] = t1;

59 } else {

69 - t2 = $[6];

60 + t1 = $[4];

61 }

71 - return t2;

62 + return t1;

63 }

65 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/todo.useContext-mutate-context-in-callback.js new

+15

@@ -0,0 +1,15 @@

1 +function Component(props) {

2 + const FooContext = useContext(Foo);

3 + // This function should be memoized, but its mutable range is entangled

4 + // with the useContext call. We can't memoize hooks, therefore the

5 + // reactive scope around the hook + callback is pruned and we're left

6 + // w no memoization of the callback.

7 + //

8 + // Ideally we'd determine that this isn't called during render and can

9 + // therefore be considered "immutable" or otherwise safe to memoize

10 + // independently

11 + const onClick = () => {

12 + FooContext.current = true;

13 + };

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

15 +}

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.expect.md new

+74

@@ -0,0 +1,74 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +import * as React from "react";

6 +import { useContext } from "react";

7 +import { mutate } from "shared-runtime";

8 +

9 +const FooContext = React.createContext({ current: null });

10 +

11 +function Component(props) {

12 + const Foo = useContext(FooContext);

13 + // This callback can be memoized because we aren't 100% positive that

14 + // `mutate()` actually mutates, so we optimistically assume it doesn't

15 + // Its range doesn't get entagled w the useContext call so we're able

16 + // to create a reactive scope and memoize it.

17 + const onClick = () => {

18 + mutate(Foo.current);

19 + };

20 + return <div onClick={onClick}>{props.children}</div>;

21 +}

22 +

23 +export const FIXTURE_ENTRYPOINT = {

24 + fn: Component,

25 + params: [{ children: <div>Hello</div> }],

26 +};

27 +

28 +```

29 +

30 +## Code

31 +

32 +```javascript

33 +import * as React from "react";

34 +import { useContext, unstable_useMemoCache as useMemoCache } from "react";

35 +import { mutate } from "shared-runtime";

36 +

37 +const FooContext = React.createContext({ current: null });

38 +

39 +function Component(props) {

40 + const $ = useMemoCache(5);

41 + const Foo = useContext(FooContext);

42 + const c_0 = $[0] !== Foo.current;

43 + let t0;

44 + if (c_0) {

45 + t0 = () => {

46 + mutate(Foo.current);

47 + };

48 + $[0] = Foo.current;

49 + $[1] = t0;

50 + } else {

51 + t0 = $[1];

52 + }

53 + const onClick = t0;

54 + const c_2 = $[2] !== onClick;

55 + const c_3 = $[3] !== props.children;

56 + let t1;

57 + if (c_2 || c_3) {

58 + t1 = <div onClick={onClick}>{props.children}</div>;

59 + $[2] = onClick;

60 + $[3] = props.children;

61 + $[4] = t1;

62 + } else {

63 + t1 = $[4];

64 + }

65 + return t1;

66 +}

67 +

68 +export const FIXTURE_ENTRYPOINT = {

69 + fn: Component,

70 + params: [{ children: <div>Hello</div> }],

71 +};

72 +

73 +```

74 +

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.js new

+22

@@ -0,0 +1,22 @@

1 +import * as React from "react";

2 +import { useContext } from "react";

3 +import { mutate } from "shared-runtime";

4 +

5 +const FooContext = React.createContext({ current: null });

6 +

7 +function Component(props) {

8 + const Foo = useContext(FooContext);

9 + // This callback can be memoized because we aren't 100% positive that

10 + // `mutate()` actually mutates, so we optimistically assume it doesn't

11 + // Its range doesn't get entagled w the useContext call so we're able

12 + // to create a reactive scope and memoize it.

13 + const onClick = () => {

14 + mutate(Foo.current);

15 + };

16 + return <div onClick={onClick}>{props.children}</div>;

17 +}

18 +

19 +export const FIXTURE_ENTRYPOINT = {

20 + fn: Component,

21 + params: [{ children: <div>Hello</div> }],

22 +};

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-mutable-value.expect.md deleted

-48

@@ -1,48 +0,0 @@

1 -

2 -## Input

3 -

4 -```javascript

5 -// @debug

6 -function Component(props) {

7 - const FooContext = useContext(Foo);

8 - const onClick = () => {

9 - FooContext.current = true;

10 - };

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

12 -}

13 -

14 -```

15 -

16 -## Code

17 -

18 -```javascript

19 -import { unstable_useMemoCache as useMemoCache } from "react"; // @debug

20 -function Component(props) {

21 - const $ = useMemoCache(4);

22 - const FooContext = useContext(Foo);

23 - const c_0 = $[0] !== FooContext.current;

24 - let t0;

25 - if (c_0) {

26 - t0 = () => {

27 - FooContext.current = true;

28 - };

29 - $[0] = FooContext.current;

30 - $[1] = t0;

31 - } else {

32 - t0 = $[1];

33 - }

34 - const onClick = t0;

35 - const c_2 = $[2] !== onClick;

36 - let t1;

37 - if (c_2) {

38 - t1 = <div onClick={onClick} />;

39 - $[2] = onClick;

40 - $[3] = t1;

41 - } else {

42 - t1 = $[3];

43 - }

44 - return t1;

45 -}

46 -

47 -```

48 -

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-mutable-value.js deleted

-8

@@ -1,8 +0,0 @@

1 -// @debug

2 -function Component(props) {

3 - const FooContext = useContext(Foo);

4 - const onClick = () => {

5 - FooContext.current = true;

6 - };

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

8 -}

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-read-context-in-callback-if-condition.expect.md new

+89

@@ -0,0 +1,89 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +import { createContext, useContext } from "react";

6 +

7 +const FooContext = createContext({ current: true });

8 +

9 +function Component(props) {

10 + const foo = useContext(FooContext);

11 +

12 + const getValue = () => {

13 + if (foo.current) {

14 + return {};

15 + } else {

16 + return null;

17 + }

18 + };

19 + const value = getValue();

20 +

21 + return <Child value={value} />;

22 +}

23 +

24 +export const FIXTURE_ENTRYPOINT = {

25 + fn: Component,

26 + params: [{}],

27 +};

28 +

29 +```

30 +

31 +## Code

32 +

33 +```javascript

34 +import {

35 + createContext,

36 + useContext,

37 + unstable_useMemoCache as useMemoCache,

38 +} from "react";

39 +

40 +const FooContext = createContext({ current: true });

41 +

42 +function Component(props) {

43 + const $ = useMemoCache(6);

44 + const foo = useContext(FooContext);

45 + const c_0 = $[0] !== foo.current;

46 + let t0;

47 + if (c_0) {

48 + t0 = () => {

49 + if (foo.current) {

50 + return {};

51 + } else {

52 + return null;

53 + }

54 + };

55 + $[0] = foo.current;

56 + $[1] = t0;

57 + } else {

58 + t0 = $[1];

59 + }

60 + const getValue = t0;

61 + const c_2 = $[2] !== getValue;

62 + let t1;

63 + if (c_2) {

64 + t1 = getValue();

65 + $[2] = getValue;

66 + $[3] = t1;

67 + } else {

68 + t1 = $[3];

69 + }

70 + const value = t1;

71 + const c_4 = $[4] !== value;

72 + let t2;

73 + if (c_4) {

74 + t2 = <Child value={value} />;

75 + $[4] = value;

76 + $[5] = t2;

77 + } else {

78 + t2 = $[5];

79 + }

80 + return t2;

81 +}

82 +

83 +export const FIXTURE_ENTRYPOINT = {

84 + fn: Component,

85 + params: [{}],

86 +};

87 +

88 +```

89 +

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-read-context-in-callback-if-condition.js new

+23

@@ -0,0 +1,23 @@

1 +import { createContext, useContext } from "react";

2 +

3 +const FooContext = createContext({ current: true });

4 +

5 +function Component(props) {

6 + const foo = useContext(FooContext);

7 +

8 + const getValue = () => {

9 + if (foo.current) {

10 + return {};

11 + } else {

12 + return null;

13 + }

14 + };

15 + const value = getValue();

16 +

17 + return <Child value={value} />;

18 +}

19 +

20 +export const FIXTURE_ENTRYPOINT = {

21 + fn: Component,

22 + params: [{}],

23 +};

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.expect.md new

+71

@@ -0,0 +1,71 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +import { createContext, useContext } from "react";

6 +

7 +const FooContext = createContext({ current: null });

8 +

9 +function Component(props) {

10 + const foo = useContext(FooContext);

11 + // This function should be memoized since it is only reading the context value

12 + const onClick = () => {

13 + console.log(foo.current);

14 + };

15 + return <div onClick={onClick}>{props.children}</div>;

16 +}

17 +

18 +export const FIXTURE_ENTRYPOINT = {

19 + fn: Component,

20 + params: [{ children: <div>Hello</div> }],

21 +};

22 +

23 +```

24 +

25 +## Code

26 +

27 +```javascript

28 +import {

29 + createContext,

30 + useContext,

31 + unstable_useMemoCache as useMemoCache,

32 +} from "react";

33 +

34 +const FooContext = createContext({ current: null });

35 +

36 +function Component(props) {

37 + const $ = useMemoCache(5);

38 + const foo = useContext(FooContext);

39 + const c_0 = $[0] !== foo.current;

40 + let t0;

41 + if (c_0) {

42 + t0 = () => {

43 + console.log(foo.current);

44 + };

45 + $[0] = foo.current;

46 + $[1] = t0;

47 + } else {

48 + t0 = $[1];

49 + }

50 + const onClick = t0;

51 + const c_2 = $[2] !== onClick;

52 + const c_3 = $[3] !== props.children;

53 + let t1;

54 + if (c_2 || c_3) {

55 + t1 = <div onClick={onClick}>{props.children}</div>;

56 + $[2] = onClick;

57 + $[3] = props.children;

58 + $[4] = t1;

59 + } else {

60 + t1 = $[4];

61 + }

62 + return t1;

63 +}

64 +

65 +export const FIXTURE_ENTRYPOINT = {

66 + fn: Component,

67 + params: [{ children: <div>Hello</div> }],

68 +};

69 +

70 +```

71 +

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.js new

+17

@@ -0,0 +1,17 @@

1 +import { createContext, useContext } from "react";

2 +

3 +const FooContext = createContext({ current: null });

4 +

5 +function Component(props) {

6 + const foo = useContext(FooContext);

7 + // This function should be memoized since it is only reading the context value

8 + const onClick = () => {

9 + console.log(foo.current);

10 + };

11 + return <div onClick={onClick}>{props.children}</div>;

12 +}

13 +

14 +export const FIXTURE_ENTRYPOINT = {

15 + fn: Component,

16 + params: [{ children: <div>Hello</div> }],

17 +};

compiler/packages/sprout/src/SproutTodoFilter.ts

+1 -1

@@ -460,7 +460,7 @@ const skipFilter = new Set([

460 "fbtparam-text-must-use-expression-container",

461 "fbtparam-with-jsx-fragment-value",

462 "fbt-preserve-jsxtext",

463 - "useContext-mutable-value",

463 + "todo.useContext-mutate-context-in-callback",

464 "loop-unused-let",

465 ]);

466