react icon indicating copy to clipboard operation
react copied to clipboard

[Compiler Bug]: Compiler doesn't bail out when reading or writing `ref.current` during a render

Open jthemphill opened this issue 1 month ago • 2 comments

What kind of issue is this?

  • [X] React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
  • [ ] babel-plugin-react-compiler (build issue installing or using the Babel plugin)
  • [ ] eslint-plugin-react-compiler (build issue installing or using the eslint plugin)
  • [ ] react-compiler-healthcheck (build issue installing or using the healthcheck script)

Link to repro

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAegFQYDoDsAEG+AYlLnAC4CWEBAJhHAEYA2jA1snoft0diACaVBCzph8AQ3wwEAMwSzc1SSxYBPfGAqTWCfACV5+CEwBWCSvgoALSRXxUJqgO6T1YPvji0wTigjK+C5UttY2+gC2ENoyloEOAG6qUPoA5lSJgQB0Al5eAAowproaWjbQYrgA5A4+kQAOVCz6TJaSUGD6ofiRUDoBznFy+HSwVLhp4QheSnSK+MgAFPkERADKFVBi094Qjc0LcPZwNuFOAPx8aHhyZJQ0BJ0I6zp6RnJLySypAJT4wG4e1wsVkIwAvPhnh8vikEL8ANxAsHZOCwJQOSHfVJIghxCiwAhg3EAXzweEwODWJHu1Foo0YrA4XGp+RARgJMBBUmGigSVFUZQQAEcoKpvIKmJI4OwTAQEFkYJo5opcms2QBVXAsKjsfQAA2eAGFJdL2PqADS7Y5qKUyhkICS4CB1OyTfQuCK4LzzBqBebkEQSU6Sd25ECrHgAFRsTkcEmedEcBEioc0DRY0sdUjgxTAEmgMD28ylXWy+AAkhIemBJAovBQIFCutb9k0WkX6YXiwhS615BBZMEENUst4WqGEHQGxEW1AGlc1jdcHdyHSnl0TbazTCbSw7ex-oC8T4QQ4wfhIc9XqUELvTTLEeS8bJORuEFv92aVnj8PgltkgGSDAaRgP84IAHzDKi6IJABQEgWBFpAn+ADaKF-sMyG-vgAC62F-k+uAkiAJJAA

Repro steps

Our codebase has a core hook called useStableRef() which breaks one of the rules of React by mutating a ref during render. This code doesn't trigger the react-hooks lint rules, nor does the new React compiler complain when encountering this code.

I'm putting this forth in the hopes that the React compiler will bail out when it encounters one of these hooks, instead of compiling these hooks and potentially breaking our codebase when we upgrade.

  • useStableRef writes to ref.current in the hook's body. This is a side effect that occurs during rendering!
  • useCallbackRef calls into useStableRef to return a non-reactive callback. In practice, we're using this as a poor-company's useEffectEvent(), until that hook is stabilized. Should this function still compile if it calls a hook that doesn't compile?

How often does this bug happen?

Every time

What version of React are you using?

React Playground 2024-05-17

jthemphill avatar May 18 '24 02:05 jthemphill

Oh, I didn't notice "validateRefAccessDuringRender": false in the EnvironmentConfig. Figured out how to enable it, but it doesn't seem to stop the compile from happening...

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAejQAgAIDcCGANgJYAm+ALggEoIBmAgnImGACKzEB2A5rV6QQxkFGFAQAdLlLQAqWVMyzMAMShc4FYhC6ZSEOACNCBgNbJFyy5gkgAmsQSFSYTPkwx6QhFy1FCAJ6YYBT4xgiYtHSYEIYAVgiamBQAFpSYxK5EAO74AWDWcDpgmVS+mNnEqckpEQC2ECEeiT4UmASE4pg8xLg+AHS21tYACjCxYYHBKdDOXADkbUV1AA7EhBGGifhQYBFVmHVQoVRZzdGknLw1krrKngJCmMgAFMN3mADKM1DON5jLNYbGAAyhwFI1TIAfksaCkdHUmm0ul2CE+oXCUReHXEAEpMMBFADim1PNEALyYVFYnEIXEAbiJZP6cFgDzalNpjN0zQosF0ZO5AF8pDJ5NY1BotDo9AZjGYLB9hiBaHyYFwzmTvL5iP4gggAI5QIigwiEQz4OCmGK6BB9GBBB6CGCDO7KgCqXBIpgiAANUQBhfwWq2+gA0-zgwct1v0CFcXAgSzSvAi2Vq0g+ghWPkEGkcrnB+FTgxA7yUmAAKilMhlXKjSBldHVi0EVoRLfG3HBxqwYrBiYILXt+pgAJKuA5gfB0W4VigQKl7SMQVbrJ4y6AgopD-DLrZ0CCeCoIeZ9AEbYsIUjWVLxiJQFYwu5wrgIqXIpcIINmkOmLFRr+Mb4oSPJFBqpL0JglKouikw0PQLyAeawHcpgvL8l+P4oVabw8uhLz9ER+AwDwYD4uSAB85wsmyrSEcRpHkWGRLoQA2qx6HnCx+GYAAujx6EMlIQogEKQA

jthemphill avatar May 18 '24 02:05 jthemphill

Thanks for posting. First note that refs are fine to access outside of render — in particular, in event handlers and effects. It's okay to access refs in a callback function, so long as that callback is only called outside of render.

So in your example, useStableRef() technically violates the rules. It's a KP that we're not catching it when ValidateNoRefAccessInRender is enabled, that's part of why that validation is off by default. useCallbackRef() technically doesn't violate the rules, but there's a huge caveat — it's only safe to call the resulting callbacks from outside of render.

Calling a callback created by this hook during render would mean that (just like accessing a ref) your UI could grow stale.

I'd give it a try and see.

josephsavona avatar May 18 '24 04:05 josephsavona

🥳🥳🥳

jthemphill avatar May 29 '24 17:05 jthemphill