aurelia / router

A powerful client-side router.
MIT License
120 stars 115 forks source link

Child router keeps view alive after navigating away to parent router #617

Closed jmezach closed 6 years ago

jmezach commented 6 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: We have our application setup in such a way that we have a root route and a couple of child routers which handle navigation in various modules. This has been working well for us so far, but lately we have noticed that we are leaking quite a bit of memory. One of the things we are seeing is that when we navigate away from a view inside on of the modules (which is essentially in a child router) back to the root route, that the view that was loaded in the child router is being kept around.

Expected/desired behavior: We are expecting the view to go away entirely, rather than being kept around unnecessarily. I've tried to create a gist.run, but unfortunately that seems to be broken right now (as in I can't edit the gist at all).

bigopon commented 6 years ago

@jmezach Thanks for chasing this issue up. For reproduction example, please use this https://stackblitz.com/edit/aurelia-javascript?file=src%2Fapp.js

jmezach commented 6 years ago

@bigopon I've managed to create a small repro for the issue at https://aurelia-javascript-kqydff.stackblitz.io/. Interestingly enough I'm not seeing the same issue as we're seeing in our application. In the repro when I navigate from child back to home (using the buttons at the top) and take a memory snapshot I don't see the ChildRouter and Child components. But in our application we are actually seeing that it is still alive, and being referenced by the RouterView component. Not sure what's different here, any ideas?

bigopon commented 6 years ago

@jmezach My guess is your issue is similar to this https://github.com/aurelia/router/issues/614

Can you verify if bluebird is the issue ?

jmezach commented 6 years ago

We are not using bluebird anymore. We used to, but we've moved away from it a couple of weeks ago.

bigopon commented 6 years ago

I see your 👍 in this issue https://github.com/aurelia/templating/pull/637 but I'm not sure if you have already applied that fix. Can you apply the following if not:

// in main.js
import { ViewSlot, ShadowDOM } from 'aurelia-framework';

ViewSlot.prototype._projectionRemoveAll = function(returnToCache) {
    ShadowDOM.undistributeAll(this.projectToSlots, this);

    let children = this.children;
    let ii = children.length;

    for (let i = 0; i < ii; ++i) {
      if (returnToCache) {
        children[i].returnToCache();
      } else if (this.isAttached) {
        children[i].detached();
      }
    }

    this.children = [];
  }
jmezach commented 6 years ago

@bigopon ;) FYI @dannyBies and I are on the same team, so I'm very aware of the above fix. I've just applied it locally, but I'm still seeing the issue where our view is being kept around:

screenshot 2018-09-03 13 54 56

bigopon commented 6 years ago

Then my next guess is somewhere in your app, not at the root direct child level, you are injecting Router. Though theoretically it should be garbage-collected when the parent route is gone, I'm not so sure about it, as as long as there is a reference to that injected Router, the view persists.

davismj commented 6 years ago

@jmezach Since it seems that the issue is not intrinsic to the framework, the best thing for your team might be to reach out to either official or third party support to drill into this issue in your production app. Please reach out to me personally if you need help getting help.

jmezach commented 6 years ago

It turned out to be an issue where objects were being logged to the console, which kept elements alive unnecessarily. After fixing that we're no longer seeing the Router being kept around.