chore(compass-crud): Do not recreate HadronDocument instances in render #3073
Conversation
|
@Anemy can you pull the branch and test if it got better from your perspective? For me it more or less matches the lag that we had before changing the underlying Document component (I was manually comparing to older Compass version) |
| } = props; | ||
|
|
||
| const doc = useMemo(() => { | ||
| if (_doc.type === 'Document') { |
There was a problem hiding this comment.
Does this mean that the doc prop can already be a HadronDocument instance? When does that happen? Inside of hadron-document I've kind of tried to align the brand checking for the Document class to be a .isRoot() call, would it make sense to use that here as well?
There was a problem hiding this comment.
Ah, actually yes, that would be much better, thanks for the reminder this exists, I'll update!
In CRUD view plugin we do keep an actual instance of HadronDocument in redux store and do not create it in render so the instance is always the same and only updates when you edit the document so I was trying to kinda allow component interface to handle both use-cases: where we pass an instance that we manually manage outside or where we just have a "random" bson document that we want to pretty render somewhere in the app. I am not 100% happy with the solution, but also couldn't come up with a better approach that would require less changes tbh
There was a problem hiding this comment.
Yeah, I think it’s fine to not worry about this a lot until this code here is typescript and we have more control over what’s being passed in from where 🙂
gribnoysup
deleted the
make-sure-we-do-not-recreate-hadron-document-too-often
branch
2 participants
Max spotted an issue where editing a stage would collapse expanded elements in the documents in all the previous ones which highlighted an issue that is partially responsible for the editing performance degradation that we currently see in Compass. Creating
HadronDocumentinstances right in the render method combined with the fact that all the state inside the plugin is propagated down from the plugin root to the components through props lead to constant recreation of all the Documents in all the stages on every single change in the doc and this caused complete remounting of all components inside the Document component because Document and its children uuid was constantly changing.This patch fixes the issue by handling "raw" objects passed to the component a bit better: we check whether passed component is a HadronDocument type and only create an instance if it's not and if the underlying object changed.
There is still a somewhat noticeable lag when we actually fetch the documents and have to re-create these HadronDocument instances and update the views, but the massive slowdown that was happening on every change seems to be gone now.