chris-ware / nova-breadcrumbs

Breadcrumbs for Laravel Nova
152 stars 44 forks source link

Navigating to other resources adds previous resource's breadcrumb #125

Closed abishekrsrikaanth closed 3 years ago

abishekrsrikaanth commented 3 years ago

Describe the bug Navigating to other resources adds previous resource's breadcrumb

Nova Version: 3.27.0 Nova Breadcrumb version: 2.2.0

Screenshots CleanShot 2021-07-14 at 01 31 28

I initially thought if this could be a problem with the theme I am using, but I have the same issue even after going back to the original nova theme.

chris-ware commented 3 years ago

@abishekrsrikaanth I'll try and take a look at this when I can. I currently am only using this on one project at the moment, so I haven't spent much time at all with it.

abishekrsrikaanth commented 3 years ago

@chris-ware, thank you. Currently having this issue on Production though. If you can point me in the right direction, I can try to debug and figure out the issue and possibly send a PR for your review.

chris-ware commented 3 years ago

It's pretty hard to determine remotely, but if it's anywhere, it'll likely be in NovaBreadcrumbsController.php. You should be able to determine if it is or not by looking at the API response from breadcrumbs and seeing what is returned there. If it is only returning the one set of breadcrumbs, then the issue is likely in the Vue component for breadcrumbs itself. I will try my best to take a look at it, but from what I can gather from one of my colleagues, they don't have this problem on another of our projects in a production environment.

chris-ware commented 3 years ago

I have actually had chance to try this as a fresh require on a Nova site and also have no issues with it, so I don't believe at the moment that it is an issue with the package itself. The same colleague whom tested this on one of our production sites did raise that it could be Vue not properly clearing up dead components, but that's a bit beyond my understanding. For now, I'll close this issue until I have steps to replicate.

Something I've seen done in other repositories that I like the idea of is being provided a plain github repo that one can start up and instantly get the bug, so if you are able to provide something like that (not your actual project github repo), that'd be great.

abishekrsrikaanth commented 3 years ago

I tried to debug a bit and the nova-breadcrumbs (nova-vendor/chris-ware/nova-breadcrumbs) endpoint that nova does returns the following output, i am not quite sure why it does this. I am not quite sure if this has something to do with some sort of caching. I have redis cache enabled on both my local and production environment. Can I do anything else to figure this out?

CleanShot 2021-07-28 at 08 39 34@2x
chris-ware commented 3 years ago

On those requests, you should be able to see the data posted to the API, specifically the location array. Could you see what that is sending? They get passed and processed by https://github.com/chris-ware/nova-breadcrumbs/blob/master/src/Http/Controllers/NovaBreadcrumbsController.php which shouldn't return any more than a few items. There isn't anything in there that cycles through the path parts, it all runs on identifying how many items and what the items contain. In your case, it's almost as if invoke is being called multiple times, all passing in the data.

Caching shouldn't affect it as the controller doesn't cache anything.

Have you got any other packages for Nova that could be impacting it?

As I've said above, unfortunately, if I can't replicate it, I can't fix it... I can't even determine if it's an issue with my package. I'll try and help as much as I can, but there are limits of what I can do.

abishekrsrikaanth commented 3 years ago

@chris-ware, I additional detail I missed mentioning is that I have this issue when running this application using Octane. Any chance you can try to reproduce this with Octane?

chris-ware commented 3 years ago

I have yet to use Octane, but I can see why that might cause this issue. I'll have to try and see, but I can't make any promises.

abishekrsrikaanth commented 3 years ago

Thanks, look forward to it, I had to remove this package for a few of my clients who I migrated to Octane, will wait for the fix so I can add the package back into my projects.

steven-fox commented 2 years ago

@chris-ware @abishekrsrikaanth

I apologize for resurrecting an old issue, however, I found the solution to this problem (aka, when using Octane, the breadcrumbs from each request are appended).

When using Octane, the NovaBreadcrumbsController is only __constructed once per worker (at least when using Swoole - I haven't tested with RoadRunner) and then held in memory. Thus, when the same worker responds to subsequent requests, the $crumbs property on the controller isn't reset to an empty collection.

The solution is as simple as removing the __construct() function and adding $this->crumbs = new Collection(); to the start of the __invoke() method. This will ensure the crumbs are reset on each request. I have tested locally and it's working well. I also believe this solution will have no changes for non-Octane users.

I will whip up a PR.

chris-ware commented 2 years ago

Excellent, thanks @steven-fox. I haven't had any time or need to maintain this package for now, as I'm only using Nova for one project now, as we've moved onto custom coded solutions that better fit the needs.

I do still plan on working on V3 at some point, but its way down on my priority list.

steven-fox commented 2 years ago

You're welcome, @chris-ware. The solving PR (#129) is ready for your review. Do you believe you'll have a moment available to review, merge, and release a minor version soon, or shall I incorporate my forked version into our composer list? Either option is fine with me - I totally understand not having the time to maintain a package that is seldom used by the owner.