dbeaver/pro#8333 fix: Eliminate React 19 error in useEditorDefaultExtensions#4381
dbeaver/pro#8333 fix: Eliminate React 19 error in useEditorDefaultExtensions#4381SychevAndrey wants to merge 1 commit into
Conversation
…ensions replace useRef with useState for managing editor options state
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 4 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| extensions.current = createExtensions(options); | ||
| if (!isObjectsEqual(options, state.options)) { | ||
| const result = createExtensions(options); | ||
| setState({ options, result }); |
There was a problem hiding this comment.
this will cause extra rerenders, its better to comment such code parts as expected than fix it this way
There was a problem hiding this comment.
how many? how will it affect the product?
There was a problem hiding this comment.
Props are changed -> rerender, then if options are different, you are setting state that rerenders one more time and its just a bad react practice. You are executing state update from component body to sync state from props, it might cause infinite rerenders and other problems. I think that the prev approach is cleaner and less buggy. If we know what we are doing, we dont need to fix it. I suggest to disable this eslint rule as this pattern is commonly used across react ecosystem, what do you think?
| extensions.current = createExtensions(options); | ||
| if (!isObjectsEqual(options, state.options)) { | ||
| const result = createExtensions(options); | ||
| setState({ options, result }); |
There was a problem hiding this comment.
Props are changed -> rerender, then if options are different, you are setting state that rerenders one more time and its just a bad react practice. You are executing state update from component body to sync state from props, it might cause infinite rerenders and other problems. I think that the prev approach is cleaner and less buggy. If we know what we are doing, we dont need to fix it. I suggest to disable this eslint rule as this pattern is commonly used across react ecosystem, what do you think?
3 participants
replace useRef with useState for managing editor options state