| 1 | /** |
| 2 | * Copyright (c) Meta Platforms, Inc. and affiliates. |
| 3 | * |
| 4 | * This source code is licensed under the MIT license found in the |
| 5 | * LICENSE file in the root directory of this source tree. |
| 6 | */ |
| 7 | |
| 8 | import { |
| 9 | CompilerDiagnostic, |
| 10 | CompilerError, |
| 11 | ErrorCategory, |
| 12 | } from '../CompilerError'; |
| 13 | import { |
| 14 | Environment, |
| 15 | HIRFunction, |
| 16 | IdentifierId, |
| 17 | isSetStateType, |
| 18 | isUseEffectHookType, |
| 19 | isUseEffectEventType, |
| 20 | isUseInsertionEffectHookType, |
| 21 | isUseLayoutEffectHookType, |
| 22 | isUseRefType, |
| 23 | isRefValueType, |
| 24 | Place, |
| 25 | Effect, |
| 26 | BlockId, |
| 27 | } from '../HIR'; |
| 28 | import { |
| 29 | eachInstructionLValue, |
| 30 | eachInstructionValueOperand, |
| 31 | } from '../HIR/visitors'; |
| 32 | import {createControlDominators} from '../Inference/ControlDominators'; |
| 33 | import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; |
| 34 | import {Result} from '../Utils/Result'; |
| 35 | import {assertExhaustive, Iterable_some} from '../Utils/utils'; |
| 36 | |
| 37 | /** |
| 38 | * Validates against calling setState in the body of an effect (useEffect and friends), |
| 39 | * while allowing calling setState in callbacks scheduled by the effect. |
| 40 | * |
| 41 | * Calling setState during execution of a useEffect triggers a re-render, which is |
| 42 | * often bad for performance and frequently has more efficient and straightforward |
| 43 | * alternatives. See https://react.dev/learn/you-might-not-need-an-effect for examples. |
| 44 | */ |
| 45 | export function validateNoSetStateInEffects( |
| 46 | fn: HIRFunction, |
| 47 | env: Environment, |
| 48 | ): Result<void, CompilerError> { |
| 49 | const setStateFunctions: Map<IdentifierId, Place> = new Map(); |
| 50 | const errors = new CompilerError(); |
| 51 | for (const [, block] of fn.body.blocks) { |
| 52 | for (const instr of block.instructions) { |
| 53 | switch (instr.value.kind) { |
| 54 | case 'LoadLocal': { |
| 55 | if (setStateFunctions.has(instr.value.place.identifier.id)) { |
| 56 | setStateFunctions.set( |
| 57 | instr.lvalue.identifier.id, |
| 58 | instr.value.place, |
| 59 | ); |
| 60 | } |
| 61 | break; |
| 62 | } |
| 63 | case 'StoreLocal': { |
| 64 | if (setStateFunctions.has(instr.value.value.identifier.id)) { |
| 65 | setStateFunctions.set( |
| 66 | instr.value.lvalue.place.identifier.id, |
| 67 | instr.value.value, |
| 68 | ); |
| 69 | setStateFunctions.set( |
| 70 | instr.lvalue.identifier.id, |
| 71 | instr.value.value, |
| 72 | ); |
| 73 | } |
| 74 | break; |
| 75 | } |
| 76 | case 'FunctionExpression': { |
| 77 | if ( |
| 78 | // faster-path to check if the function expression references a setState |
| 79 | [...eachInstructionValueOperand(instr.value)].some( |
| 80 | operand => |
| 81 | isSetStateType(operand.identifier) || |
| 82 | setStateFunctions.has(operand.identifier.id), |
| 83 | ) |
| 84 | ) { |
| 85 | const callee = getSetStateCall( |
| 86 | instr.value.loweredFunc.func, |
| 87 | setStateFunctions, |
| 88 | env, |
| 89 | ); |
| 90 | if (callee !== null) { |
| 91 | setStateFunctions.set(instr.lvalue.identifier.id, callee); |
| 92 | } |
| 93 | } |
| 94 | break; |
| 95 | } |
| 96 | case 'MethodCall': |
| 97 | case 'CallExpression': { |
| 98 | const callee = |
| 99 | instr.value.kind === 'MethodCall' |
| 100 | ? instr.value.property |
| 101 | : instr.value.callee; |
| 102 | |
| 103 | if (isUseEffectEventType(callee.identifier)) { |
| 104 | const arg = instr.value.args[0]; |
| 105 | if (arg !== undefined && arg.kind === 'Identifier') { |
| 106 | const setState = setStateFunctions.get(arg.identifier.id); |
| 107 | if (setState !== undefined) { |
| 108 | /** |
| 109 | * This effect event function calls setState synchonously, |
| 110 | * treat it as a setState function for transitive tracking |
| 111 | */ |
| 112 | setStateFunctions.set(instr.lvalue.identifier.id, setState); |
| 113 | } |
| 114 | } |
| 115 | } else if ( |
| 116 | isUseEffectHookType(callee.identifier) || |
| 117 | isUseLayoutEffectHookType(callee.identifier) || |
| 118 | isUseInsertionEffectHookType(callee.identifier) |
| 119 | ) { |
| 120 | const arg = instr.value.args[0]; |
| 121 | if (arg !== undefined && arg.kind === 'Identifier') { |
| 122 | const setState = setStateFunctions.get(arg.identifier.id); |
| 123 | if (setState !== undefined) { |
| 124 | const enableVerbose = |
| 125 | env.config.enableVerboseNoSetStateInEffect; |
| 126 | if (enableVerbose) { |
| 127 | errors.pushDiagnostic( |
| 128 | CompilerDiagnostic.create({ |
| 129 | category: ErrorCategory.EffectSetState, |
| 130 | reason: |
| 131 | 'Calling setState synchronously within an effect can trigger cascading renders', |
| 132 | description: |
| 133 | 'Effects are intended to synchronize state between React and external systems. ' + |
| 134 | 'Calling setState synchronously causes cascading renders that hurt performance.\n\n' + |
| 135 | 'This pattern may indicate one of several issues:\n\n' + |
| 136 | '**1. Non-local derived data**: If the value being set could be computed from props/state ' + |
| 137 | 'but requires data from a parent component, consider restructuring state ownership so the ' + |
| 138 | 'derivation can happen during render in the component that owns the relevant state.\n\n' + |
| 139 | "**2. Derived event pattern**: If you're detecting when a prop changes (e.g., `isPlaying` " + |
| 140 | 'transitioning from false to true), this often indicates the parent should provide an event ' + |
| 141 | 'callback (like `onPlay`) instead of just the current state. Request access to the original event.\n\n' + |
| 142 | "**3. Force update / external sync**: If you're forcing a re-render to sync with an external " + |
| 143 | 'data source (mutable values outside React), use `useSyncExternalStore` to properly subscribe ' + |
| 144 | 'to external state changes.\n\n' + |
| 145 | 'See: https://react.dev/learn/you-might-not-need-an-effect', |
| 146 | suggestions: null, |
| 147 | }).withDetails({ |
| 148 | kind: 'error', |
| 149 | loc: setState.loc, |
| 150 | message: |
| 151 | 'Avoid calling setState() directly within an effect', |
| 152 | }), |
| 153 | ); |
| 154 | } else { |
| 155 | errors.pushDiagnostic( |
| 156 | CompilerDiagnostic.create({ |
| 157 | category: ErrorCategory.EffectSetState, |
| 158 | reason: |
| 159 | 'Calling setState synchronously within an effect can trigger cascading renders', |
| 160 | description: |
| 161 | 'Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. ' + |
| 162 | 'In general, the body of an effect should do one or both of the following:\n' + |
| 163 | '* Update external systems with the latest state from React.\n' + |
| 164 | '* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\n' + |
| 165 | 'Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. ' + |
| 166 | '(https://react.dev/learn/you-might-not-need-an-effect)', |
| 167 | suggestions: null, |
| 168 | }).withDetails({ |
| 169 | kind: 'error', |
| 170 | loc: setState.loc, |
| 171 | message: |
| 172 | 'Avoid calling setState() directly within an effect', |
| 173 | }), |
| 174 | ); |
| 175 | } |
| 176 | } |
| 177 | } |
| 178 | } |
| 179 | break; |
| 180 | } |
| 181 | } |
| 182 | } |
| 183 | } |
| 184 | |
| 185 | return errors.asResult(); |
| 186 | } |
| 187 | |
| 188 | function getSetStateCall( |
| 189 | fn: HIRFunction, |
| 190 | setStateFunctions: Map<IdentifierId, Place>, |
| 191 | env: Environment, |
| 192 | ): Place | null { |
| 193 | const enableAllowSetStateFromRefsInEffects = |
| 194 | env.config.enableAllowSetStateFromRefsInEffects; |
| 195 | const refDerivedValues: Set<IdentifierId> = new Set(); |
| 196 | |
| 197 | const isDerivedFromRef = (place: Place): boolean => { |
| 198 | return ( |
| 199 | refDerivedValues.has(place.identifier.id) || |
| 200 | isUseRefType(place.identifier) || |
| 201 | isRefValueType(place.identifier) |
| 202 | ); |
| 203 | }; |
| 204 | |
| 205 | const isRefControlledBlock: (id: BlockId) => boolean = |
| 206 | enableAllowSetStateFromRefsInEffects |
| 207 | ? createControlDominators(fn, place => isDerivedFromRef(place)) |
| 208 | : (): boolean => false; |
| 209 | |
| 210 | for (const [, block] of fn.body.blocks) { |
| 211 | if (enableAllowSetStateFromRefsInEffects) { |
| 212 | for (const phi of block.phis) { |
| 213 | if (isDerivedFromRef(phi.place)) { |
| 214 | continue; |
| 215 | } |
| 216 | let isPhiDerivedFromRef = false; |
| 217 | for (const [, operand] of phi.operands) { |
| 218 | if (isDerivedFromRef(operand)) { |
| 219 | isPhiDerivedFromRef = true; |
| 220 | break; |
| 221 | } |
| 222 | } |
| 223 | if (isPhiDerivedFromRef) { |
| 224 | refDerivedValues.add(phi.place.identifier.id); |
| 225 | } else { |
| 226 | for (const [pred] of phi.operands) { |
| 227 | if (isRefControlledBlock(pred)) { |
| 228 | refDerivedValues.add(phi.place.identifier.id); |
| 229 | break; |
| 230 | } |
| 231 | } |
| 232 | } |
| 233 | } |
| 234 | } |
| 235 | for (const instr of block.instructions) { |
| 236 | if (enableAllowSetStateFromRefsInEffects) { |
| 237 | const hasRefOperand = Iterable_some( |
| 238 | eachInstructionValueOperand(instr.value), |
| 239 | isDerivedFromRef, |
| 240 | ); |
| 241 | |
| 242 | if (hasRefOperand) { |
| 243 | for (const lvalue of eachInstructionLValue(instr)) { |
| 244 | refDerivedValues.add(lvalue.identifier.id); |
| 245 | } |
| 246 | // Ref-derived values can also propagate through mutation |
| 247 | for (const operand of eachInstructionValueOperand(instr.value)) { |
| 248 | switch (operand.effect) { |
| 249 | case Effect.Capture: |
| 250 | case Effect.Store: |
| 251 | case Effect.ConditionallyMutate: |
| 252 | case Effect.ConditionallyMutateIterator: |
| 253 | case Effect.Mutate: { |
| 254 | if (isMutable(instr, operand)) { |
| 255 | refDerivedValues.add(operand.identifier.id); |
| 256 | } |
| 257 | break; |
| 258 | } |
| 259 | case Effect.Freeze: |
| 260 | case Effect.Read: { |
| 261 | // no-op |
| 262 | break; |
| 263 | } |
| 264 | case Effect.Unknown: { |
| 265 | CompilerError.invariant(false, { |
| 266 | reason: 'Unexpected unknown effect', |
| 267 | loc: operand.loc, |
| 268 | }); |
| 269 | } |
| 270 | default: { |
| 271 | assertExhaustive( |
| 272 | operand.effect, |
| 273 | `Unexpected effect kind \`${operand.effect}\``, |
| 274 | ); |
| 275 | } |
| 276 | } |
| 277 | } |
| 278 | } |
| 279 | |
| 280 | if ( |
| 281 | instr.value.kind === 'PropertyLoad' && |
| 282 | instr.value.property === 'current' && |
| 283 | (isUseRefType(instr.value.object.identifier) || |
| 284 | isRefValueType(instr.value.object.identifier)) |
| 285 | ) { |
| 286 | refDerivedValues.add(instr.lvalue.identifier.id); |
| 287 | } |
| 288 | } |
| 289 | |
| 290 | switch (instr.value.kind) { |
| 291 | case 'LoadLocal': { |
| 292 | if (setStateFunctions.has(instr.value.place.identifier.id)) { |
| 293 | setStateFunctions.set( |
| 294 | instr.lvalue.identifier.id, |
| 295 | instr.value.place, |
| 296 | ); |
| 297 | } |
| 298 | break; |
| 299 | } |
| 300 | case 'StoreLocal': { |
| 301 | if (setStateFunctions.has(instr.value.value.identifier.id)) { |
| 302 | setStateFunctions.set( |
| 303 | instr.value.lvalue.place.identifier.id, |
| 304 | instr.value.value, |
| 305 | ); |
| 306 | setStateFunctions.set( |
| 307 | instr.lvalue.identifier.id, |
| 308 | instr.value.value, |
| 309 | ); |
| 310 | } |
| 311 | break; |
| 312 | } |
| 313 | case 'CallExpression': { |
| 314 | const callee = instr.value.callee; |
| 315 | if ( |
| 316 | isSetStateType(callee.identifier) || |
| 317 | setStateFunctions.has(callee.identifier.id) |
| 318 | ) { |
| 319 | if (enableAllowSetStateFromRefsInEffects) { |
| 320 | const arg = instr.value.args.at(0); |
| 321 | if ( |
| 322 | arg !== undefined && |
| 323 | arg.kind === 'Identifier' && |
| 324 | refDerivedValues.has(arg.identifier.id) |
| 325 | ) { |
| 326 | /** |
| 327 | * The one special case where we allow setStates in effects is in the very specific |
| 328 | * scenario where the value being set is derived from a ref. For example this may |
| 329 | * be needed when initial layout measurements from refs need to be stored in state. |
| 330 | */ |
| 331 | return null; |
| 332 | } else if (isRefControlledBlock(block.id)) { |
| 333 | continue; |
| 334 | } |
| 335 | } |
| 336 | /* |
| 337 | * TODO: once we support multiple locations per error, we should link to the |
| 338 | * original Place in the case that setStateFunction.has(callee) |
| 339 | */ |
| 340 | return callee; |
| 341 | } |
| 342 | } |
| 343 | } |
| 344 | } |
| 345 | } |
| 346 | return null; |
| 347 | } |