feat: performance tweaks to plugin-nested-docs [v3] #5706
Conversation
cuquo
requested review from
AlessioGr,
DanRibbens,
JarrodMFlesch,
denolfe,
jacobsfletch,
jessrynkar and
jmikrut
as code owners
| if (!originalDoc) return true | ||
|
|
||
| // if it's a nested collection, we should populate the breadcrumbs | ||
| if (data?.hierarchy && !Array.isArray(data?.hierarchy)) { |
There was a problem hiding this comment.
sorry, it should take the value from the breadcrumbsFieldSlug from the config
| own `parent` field to each collection manually. This gives you complete control over where you put the field in your | ||
| admin dashboard, etc. Set this property to the `name` of your custom field. | ||
|
|
||
| #### `urlFieldSlug` |
There was a problem hiding this comment.
It took me a while as I reviewed this PR to understand fully what this new property is for. I am not sure I do, even now.
If we want to compare a new URL vs. an old URL, to determine if we need to recreate breadcrumbs, could we not do that by looking at the last breadcrumb in the array of breadcrumbs?
The generateURL function could be used to create the new URL, and then we use that newly created URL to compare against the breadcrumbs coming in from the previous doc. Right?
I would like to avoid having another property introduced into the config of this plugin.
Let me know what you think!
There was a problem hiding this comment.
what happens right now, is that the nested-doc plugin will always be executed when something has change from a nested collection. It's not an issue with a low amount of items within a collection but when you have hundreds or thousand of its, it will run across all of them and their children.
Inside shouldPopulateBreadcrumbs we check if the collection key has changed or its new, there's also generateURL as an optional prop for the plugin, so we need to populate urlFieldSlug with the value it would take in order to also avoid doing the whole process if that value changes otherwise it will only not save when the collection key is different.
|
This PR is stale due to lack of activity. To keep the PR open, please indicate that it is still relevant in a comment below. |
3 participants
Description
Type of change
Checklist: