MobX-React: Remove need for observer
HOC, like Preact Signals & Legend State
#3528
Replies: 7 comments 8 replies
-
Initially I thought it's yet another "let's use hooks instead of observer" topic discussed here dozen of times, but then I realized it's something more interesting. Thanks for bringing this up! Unfortunately I am unable to provide qualified feedback here, so let's wait for other maintainers. Am I understanding correctly that this patch is going to affect even vendor code (like Antd table or Material UI dropdown), so we won't need toJS for such cases anymore? |
Beta Was this translation helpful? Give feedback.
-
Interesting share : ) |
Beta Was this translation helpful? Give feedback.
-
The primary concern I'd have here is that it would affect any React component (as far as I could tell from the code), including components from libraries etc. That sounds potentially quite dangerous leading to subtle bugs, as MobX introduces several assumptions, like applying |
Beta Was this translation helpful? Give feedback.
-
Author of Legend-State here with some technical details. Happy to share any more details if I can help. The downside of the In practice I'm not sure what is better for performance - inject a useReducer into every component vs. the I don't think there's a way to detect file path in there. We can also track the ReactCurrentOwner, but it doesn't have much information beyond the props. If you want to do that I think you'd want to go a direction of making a babel plugin that auto-wraps components in But it leaves If you're seriously looking into this, I'd suggest also looking at https://github.com/preactjs/signals/blob/main/packages/react/src/index.ts. That's what inspired my solution, but it has a different approach which also tracks the current owner fiber. |
Beta Was this translation helpful? Give feedback.
-
You would either loose |
Beta Was this translation helpful? Give feedback.
-
Just as a side note, using anonymous hoc functions also silently breaks some eslint rules, like the React hooks one: facebook/react#20499 For this reason we split all components into two: const ProfileAvatarComp = () => ...
export const ProfileAvatar = observer(ProfileAvatarComp); Also remember the unobserved component working better with forwardRef and tools like Storybook. |
Beta Was this translation helpful? Give feedback.
-
I've actually removed this behavior from Legend-State and I recommend you don't pursue it either. We found it to be extremely error prone, completely crashing apps with mismatched number of hooks errors. The errors are easy to reproduce in Next.js with just Also, the React team has asked me not to do it and said it is likely to change in a future version. So all in all it seems like a no-go. |
Beta Was this translation helpful? Give feedback.
-
Inspired by this tweet:
https://twitter.com/jmeistrich/status/1569644918464978945
It would be great to have a way to hook into React internals to remove the need for the
observer
HOC.The way it seems to work is to hook into ReactCurrentDispatcher.current changing to detect the beginning/end of render, as described here: https://twitter.com/jmeistrich/status/1569644922382467072
Here's the relevant
enableLegendStateReact
function: https://github.com/LegendApp/legend-state/blob/65a4ae8681f098e9b529dd9790d420ee980ff26c/src/react/enableLegendStateReact.ts#L22In React Native apps, if you export a component like this:
... it'll lose the name of the component, so debugging is difficult. To fix this, we've settled on this awkward syntax:
However, if we implemented the above idea, it would simply look like this:
Thoughts?
Beta Was this translation helpful? Give feedback.
All reactions