diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 8b82e685b0bd..911f71329b44 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -748,10 +748,14 @@ function applyEffect( case 'Alias': case 'Capture': { CompilerError.invariant( - effect.kind === 'Capture' || initialized.has(effect.into.identifier.id), + effect.kind === 'Capture' || + effect.kind === 'MaybeAlias' || + initialized.has(effect.into.identifier.id), { - reason: `Expected destination value to already be initialized within this instruction for Alias effect`, - description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`, + reason: `Expected destination to already be initialized within this instruction`, + description: + `Destination ${printPlace(effect.into)} is not initialized in this ` + + `instruction for effect ${printAliasingEffect(effect)}`, details: [ { kind: 'error', @@ -767,49 +771,67 @@ function applyEffect( * copy-on-write semantics, then we can prune the effect */ const intoKind = state.kind(effect.into).kind; - let isMutableDesination: boolean; + let destinationType: 'context' | 'mutable' | null = null; switch (intoKind) { - case ValueKind.Context: - case ValueKind.Mutable: - case ValueKind.MaybeFrozen: { - isMutableDesination = true; + case ValueKind.Context: { + destinationType = 'context'; break; } - default: { - isMutableDesination = false; + case ValueKind.Mutable: + case ValueKind.MaybeFrozen: { + destinationType = 'mutable'; break; } } const fromKind = state.kind(effect.from).kind; - let isMutableReferenceType: boolean; + let sourceType: 'context' | 'mutable' | 'frozen' | null = null; switch (fromKind) { + case ValueKind.Context: { + sourceType = 'context'; + break; + } case ValueKind.Global: case ValueKind.Primitive: { - isMutableReferenceType = false; break; } case ValueKind.Frozen: { - isMutableReferenceType = false; - applyEffect( - context, - state, - { - kind: 'ImmutableCapture', - from: effect.from, - into: effect.into, - }, - initialized, - effects, - ); + sourceType = 'frozen'; break; } default: { - isMutableReferenceType = true; + sourceType = 'mutable'; break; } } - if (isMutableDesination && isMutableReferenceType) { + + if (sourceType === 'frozen') { + applyEffect( + context, + state, + { + kind: 'ImmutableCapture', + from: effect.from, + into: effect.into, + }, + initialized, + effects, + ); + } else if ( + (sourceType === 'mutable' && destinationType === 'mutable') || + effect.kind === 'MaybeAlias' + ) { effects.push(effect); + } else if ( + (sourceType === 'context' && destinationType != null) || + (sourceType === 'mutable' && destinationType === 'context') + ) { + applyEffect( + context, + state, + {kind: 'MaybeAlias', from: effect.from, into: effect.into}, + initialized, + effects, + ); } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts index 32f84e1e28b2..43148dc4c67f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -779,7 +779,13 @@ class AliasingState { if (edge.index >= index) { break; } - queue.push({place: edge.node, transitive, direction: 'forwards', kind}); + queue.push({ + place: edge.node, + transitive, + direction: 'forwards', + // Traversing a maybeAlias edge always downgrades to conditional mutation + kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind, + }); } for (const [alias, when] of node.createdFrom) { if (when >= index) { @@ -807,7 +813,12 @@ class AliasingState { if (when >= index) { continue; } - queue.push({place: alias, transitive, direction: 'backwards', kind}); + queue.push({ + place: alias, + transitive, + direction: 'backwards', + kind, + }); } /** * MaybeAlias indicates potential data flow from unknown function calls, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts index eb053ac4196a..abbb7d847698 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts @@ -639,12 +639,55 @@ function validateNoRefAccessInRenderImpl( case 'StartMemoize': case 'FinishMemoize': break; + case 'LoadGlobal': { + if (instr.value.binding.name === 'undefined') { + env.set(instr.lvalue.identifier.id, {kind: 'Nullable'}); + } + break; + } case 'Primitive': { if (instr.value.value == null) { env.set(instr.lvalue.identifier.id, {kind: 'Nullable'}); } break; } + case 'UnaryExpression': { + if (instr.value.operator === '!') { + const value = env.get(instr.value.value.identifier.id); + const refId = + value?.kind === 'RefValue' && value.refId != null + ? value.refId + : null; + if (refId !== null) { + /* + * Record an error suggesting the `if (ref.current == null)` pattern, + * but also record the lvalue as a guard so that we don't emit a second + * error for the write to the ref + */ + env.set(instr.lvalue.identifier.id, {kind: 'Guard', refId}); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.Refs, + reason: 'Cannot access refs during render', + description: ERROR_DESCRIPTION, + }) + .withDetails({ + kind: 'error', + loc: instr.value.value.loc, + message: `Cannot access ref value during render`, + }) + .withDetails({ + kind: 'hint', + message: + 'To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`', + }), + ); + break; + } + } + validateNoRefValueAccess(errors, env, instr.value.value); + break; + } case 'BinaryExpression': { const left = env.get(instr.value.left.identifier.id); const right = env.get(instr.value.right.identifier.id); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.expect.md new file mode 100644 index 000000000000..c355819320af --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + if (r.current == undefined) { + r.current = 1; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +## Code + +```javascript +import { useRef } from "react"; + +function C() { + const r = useRef(null); + if (r.current == undefined) { + r.current = 1; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.js new file mode 100644 index 000000000000..886f025287bb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-initialization-undefined.js @@ -0,0 +1,14 @@ +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + if (r.current == undefined) { + r.current = 1; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.expect.md new file mode 100644 index 000000000000..ce1be800a13b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + const current = !r.current; + return
{current}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + + +## Error + +``` +Found 4 errors: + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + + 4 | component C() { + 5 | const r = useRef(null); +> 6 | const current = !r.current; + | ^^^^^^^^^ Cannot access ref value during render + 7 | return
{current}
; + 8 | } + 9 | + +To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }` + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + + 4 | component C() { + 5 | const r = useRef(null); +> 6 | const current = !r.current; + | ^^^^^^^^^^ Cannot access ref value during render + 7 | return
{current}
; + 8 | } + 9 | + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + + 5 | const r = useRef(null); + 6 | const current = !r.current; +> 7 | return
{current}
; + | ^^^^^^^ Cannot access ref value during render + 8 | } + 9 | + 10 | export const FIXTURE_ENTRYPOINT = { + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + + 5 | const r = useRef(null); + 6 | const current = !r.current; +> 7 | return
{current}
; + | ^^^^^^^ Cannot access ref value during render + 8 | } + 9 | + 10 | export const FIXTURE_ENTRYPOINT = { +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.js new file mode 100644 index 000000000000..8d99008f805b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-access-render-unary.js @@ -0,0 +1,13 @@ +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + const current = !r.current; + return
{current}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.expect.md new file mode 100644 index 000000000000..516d006c205a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + if (!r.current) { + r.current = 1; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + + 4 | component C() { + 5 | const r = useRef(null); +> 6 | if (!r.current) { + | ^^^^^^^^^ Cannot access ref value during render + 7 | r.current = 1; + 8 | } + 9 | } + +To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }` +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.js new file mode 100644 index 000000000000..b9b5d41295cd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-initialization-unary-not.js @@ -0,0 +1,14 @@ +//@flow +import {useRef} from 'react'; + +component C() { + const r = useRef(null); + if (!r.current) { + r.current = 1; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md new file mode 100644 index 000000000000..699140137ac2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md @@ -0,0 +1,82 @@ + +## Input + +```javascript +// @compilationMode:"infer" +function Component() { + const dispatch = useDispatch(); + // const [state, setState] = useState(0); + + return ( +
+ { + dispatch(...event.target); + event.target.value = ''; + }} + /> +
+ ); +} + +function useDispatch() { + 'use no memo'; + // skip compilation to make it easier to debug the above function + return (...values) => { + console.log(...values); + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer" +function Component() { + const $ = _c(2); + const dispatch = useDispatch(); + let t0; + if ($[0] !== dispatch) { + t0 = ( +
+ { + dispatch(...event.target); + event.target.value = ""; + }} + /> +
+ ); + $[0] = dispatch; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function useDispatch() { + "use no memo"; + // skip compilation to make it easier to debug the above function + return (...values) => { + console.log(...values); + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js new file mode 100644 index 000000000000..386729e40789 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js @@ -0,0 +1,30 @@ +// @compilationMode:"infer" +function Component() { + const dispatch = useDispatch(); + // const [state, setState] = useState(0); + + return ( +
+ { + dispatch(...event.target); + event.target.value = ''; + }} + /> +
+ ); +} + +function useDispatch() { + 'use no memo'; + // skip compilation to make it easier to debug the above function + return (...values) => { + console.log(...values); + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +};