Skip to content
This repository was archived by the owner on Oct 26, 2018. It is now read-only.
This repository was archived by the owner on Oct 26, 2018. It is now read-only.

Stopping cycles #50

Closed
Closed
@kimjoar

Description

@kimjoar

There are two flows to manipulate the browser history:

1. Update URL/history
   -> history callback
   -> dispatch push/replace action
   -> store callback
2. Dispatch push/replace action
   -> store callback
   -> update history
   -> history callback

In both cases we call both of our callbacks, and in both cases we need to make sure that the flow stops before we create a cycle. E.g. let's say we want to change the URL by dispatching an action. This action triggers a history update, and when handling this update we have to make sure we don't dispatch yet another time.

To solve both cases without relying on deep-equal I think we need to track two variables — one to stop each flow.

To handle case 1 we set { avoidRouterUpdate: true } in the history callback. As we don't increment the changeId we can stop the flow in the store callback by comparing it against the lastChangeId. By using changeId we also enable actions in history.listenBefore callbacks.

To handle case 2 we need to stop after we have updated the history based on a pushState or replaceState action. The problem is how to keep track of this update. In case 1 we decide to not increment the change id, thereby stopping the flow, but when updating the history I'm not sure how we can do the same. Right now we rely on comparing the current router state location with the new history location, but it would be nice to not need the deep-equal dependency in #38.

One option is to keep some kind of state in location.state in the same way as we have changeId in the store state, but I'm not sure if that would work (We don't have the same control over the history methods as we have over the pushPath and replacePath actions). Another option is to add another variable, e.g. isRoutingFromAction, which keeps track of whether or not the current route change was triggered by an action. Basically something like this:

let isRoutingFromAction = false;

const unsubscribeHistory = history.listen(location => {
  if (isRoutingFromAction) {
    // By handling this we have stopped the cycle, and
    // therefore need to reset the state.
    isRoutingFromAction = false;
    return;
  }

  // ...
});

const unsubscribeStore = store.subscribe(() => {
  const routing = getRouterState();
  if(lastChangeId === routing.changeId) return;

  isRoutingFromAction = true;

  // ...
});

With this change all tests still pass and my app still works, but as we don't run the tests in browsers I'm not entirely sure about potential edge cases (e.g. I don't use listenBefore at all in my app).

(I can create a PR with this if anyone wants to play around with it)

Maybe someone has other ideas about how to handle this?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions