There is currently a consistent history bug in the app that affects navigating the library TOC, using search filters, and the topic TOC. What can happen is that clicking on a category in the library/topic TOC successfully loads the page it's supposed to, but it does not update in the URL. Instead, the URL remains "/texts" or "/topics" for example depending on the case. I noticed that when this bug occurs, in shouldHistoryUpdate in ReaderApp.jsx, history.state and this.state referred to the same object in memory so that this.state.panels and history.state.panels were also identical objects in memory, which is why shouldHistoryUpdate returns false and the URL doesn't get updated properly. This is ultimately because of two things:
1) in handlePopState, we do not clone the state before setting this.state to popped history state.
2) In multiple places in the codebase, we mutate the state directly in the non-React way (this.state.panels = ... instead of setState), so that if this.state is the same object as history.state, we modify both this.state and history.state accidentally. For example, in ReaderApp.jsx in setPanelSet:
I considered one possible solution to this problem would be to mutate this.state.panels in a way that doesn't affect the underlying memory object by using setState properly, so that even if history.state and this.state referred to the same place in memory, it wouldn't matter. However, I thought the best solution is to change handlePopState so that it clones both state and its panels, so that downstream no engineers need to remember that this.state.panels and history.state.panels might refer to the same object. We need to clone both state and panels. If we just clone the state, the panels properties will still refer to the same objects in memory for this.state and history.state, and if we only clone the panels, when we do this.state.panels[n] = ... it will modify history.state's panels.
There is currently a consistent history bug in the app that affects navigating the library TOC, using search filters, and the topic TOC. What can happen is that clicking on a category in the library/topic TOC successfully loads the page it's supposed to, but it does not update in the URL. Instead, the URL remains "/texts" or "/topics" for example depending on the case. I noticed that when this bug occurs, in
shouldHistoryUpdatein ReaderApp.jsx,history.stateandthis.statereferred to the same object in memory so thatthis.state.panelsandhistory.state.panelswere also identical objects in memory, which is whyshouldHistoryUpdatereturns false and the URL doesn't get updated properly. This is ultimately because of two things: 1) inhandlePopState, we do not clone the state before setting this.state to popped history state. 2) In multiple places in the codebase, we mutate the state directly in the non-React way (this.state.panels = ... instead of setState), so that if this.state is the same object as history.state, we modify both this.state and history.state accidentally. For example, in ReaderApp.jsx in setPanelSet:I considered one possible solution to this problem would be to mutate this.state.panels in a way that doesn't affect the underlying memory object by using setState properly, so that even if history.state and this.state referred to the same place in memory, it wouldn't matter. However, I thought the best solution is to change handlePopState so that it clones both state and its panels, so that downstream no engineers need to remember that this.state.panels and history.state.panels might refer to the same object. We need to clone both state and panels. If we just clone the state, the panels properties will still refer to the same objects in memory for this.state and history.state, and if we only clone the panels, when we do
this.state.panels[n] = ...it will modify history.state's panels.