Add eslint rule to error on reads or writes to refs in render #24506
Conversation
| const readError = { | ||
| message: | ||
| 'Reading from refs during rendering is not allowed. See https://beta.reactjs.org/apis/useref', | ||
| }; |
There was a problem hiding this comment.
I would like some help with these error messages. I'm not sure exactly how much info we'd like to expose in the message vs in the docs link.
Also, would be nice if we could link directly to the "Pitfall" heading on that docs page, but right now it doesn't seem to have an anchor link.
|
Comparing: 547b707...18d2eea Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
|
hey, is there any interest in integrating this plugin or should I split it out into a separate package? |
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
github-actions
bot
added
the
Resolution: Stale
|
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
Summary
After some discussion on the useEvent RFC and on Twitter, I and others learned that writing to refs during render is not allowed. This is documented in the new docs, but since this seems like it could be fairly common gotcha, I thought it might be useful to add to the eslint plugin.
I added a new rule (
react-hooks/pure-render) that is meant to detect non-pure code in render functions. At the moment, it only detects reads or writes to refs, but it could be expanded later to detect other patterns as well potentially.One pattern that the rule still allows is the lazy init pattern as documented here. This is detected by checking whether the access is inside an if statement that compares the value with the initial value passed to
useRef.(Side question: is this lazy init pattern really safe all the time, or only when you're sure the ref will never be reset back to its initial value?)
How did you test this change?
Added unit tests. I also ran the plugin on the React Aria code base and found a number of places that we will need to update. Verified that they all seemed valid.