Fix “Back” button sometimes redirecting out of Mastodon

[ClearlyClaire]

This handles both pushed and replaced states by overriding the history.push and history.replace in app/javascript/mastodon/router.tsx to manage a fromMastodon field in the state. When that field is set, we know going back is possible without leaving the interface.

[ClearlyClaire]

It improves consistency but does not actually fundamentally improve things: indeed, unlike what I initially believed, #24835 does not help with situations resulting from history.replace calls, which set a key but do not add to the history stack, so goBack would be called and still result in navigating out of Mastodon

[ClearlyClaire]

To actually fix this we need to properly handle history.replace. We may want to encode in the history state whether we are allowed to go back in the history, in such a way that history.replace would push it to the state and history.push would always allow it. Not sure how to best write it though.

[ClearlyClaire]

@renchap do you happen to know how we could cleanly hook into the right places to add a bit of logic and a bit of state in router.history.replace and router.history.push as described above?

[Signez]

@renchap do you happen to know how we could cleanly hook into the right places to add a bit of logic and a bit of state in router.history.replace and router.history.push as described above?

If it can help, I had to add a few lines of logic on push calls in my recent pull request: diff. It's mainly about catching the legacy context at the UI component level, opening it there and patching the functions directly.

To be fair, this won't work any more as soon as react-router will be updated to v5+ — those versions use modern contexts that we can't monkey-patch. For those versions, we can override the history object before providing it to the router: this answer on StackOverflow illustrates that!

(Update: in the meantime, I updated my PR to use this strategy too, so it's more forward compatible. You can see the result in this commit.)

[ClearlyClaire]

@Signez thanks for the input! I think I addressed the issue for good this time by building upon your router.tsx.

This works for now, but if possible, it is probably better to define History.LocationState globally to be MastodonLocalState | undefined, which I have not managed to do (I am very new to Typescript)

[renchap]

This works for now, but if possible, it is probably better to define History.LocationState globally to be MastodonLocalState | undefined, which I have not managed to do (I am very new to Typescript)

I think you can achieve it with something like this in types/react_router.d.ts:

import 'history';

interface MastodonLocationState {
  fromMastodon?: boolean;
  mastodonModalKey?: string;
}

declare module 'history' {
  namespace History {
    export type LocationState = MastodonLocationState;
  }

  export type LocationState = MastodonLocationState;
}

But it is not really useful at the moment, as the context can not be typed and React Router does not expose correct Prop Types.

This can be improved later, when we upgrade to React Router 5 and start using React Router in Typescript.

[ClearlyClaire]

I think you can achieve it with something like this in types/react_router.d.ts:

That does not seem to work, History.LocationState is still considered to be of type unknown.