Garbage collection when using the router

[anoblet]

When using routingPlugin and defining routes as such:

  "/": {
    name: "page-one",
    view: html`<page-one></page-one>`,
  },
  "/": {
    name: "page-two",
    view: html`<page-two></page-two>`,
  },

changing routes does not destroy the instance of the previous route. Navigating back and forth between each route multiple times triggers mapState to run on multiple instances of the same route.

The only solution I've found is to add

if(!this.isConnected) return;

to the top of mapState.

[CaptainCodeman]

You haven't show how you are rendering the views (the router outlet). What is and isn't connected to the DOM should depend on that.

[anoblet]

import { customElement, property } from "lit-element";
import { Connected, Route, RouteSelectors, State } from "./connected";

declare global {
  interface HTMLElementTagNameMap {
    "app-view": AppViewElement;
  }
}

@customElement("app-view")
export class AppViewElement extends Connected {
  @property({ type: Object })
  route: Route;

  mapState(state: State) {
    return {
      route: RouteSelectors.route(state),
    };
  }

  render() {
    return this.route.view;
  }
}

[CaptainCodeman]

It's actually a bug in the connect mixin that was trying to remove the event listener from itself instead of from the store. I'll publish an update soon to resolve it.

For the routing itself, I wouldn't generally recommend this option. Although it's convenient, it only really saves adding a switch mapping from the view name to the view html in the router outlet but it does have some implications around optimizations. Because the state store now depends on the html view templating it won't be quite as small as it otherwise could be. I prefer to try to keep it tiny so that it can initiate the routing, auth and any data requests as soon as possible while the UI may still be loading (those pieces tend to be significantly bigger).

Really depends on your approach to code-splitting though - I can see there are fewer moving parts having the views defined alongside the routes.

[CaptainCodeman]

Updated - thanks for catching this!