Upgrade to react-router v5

[renchap]

Like all my in-progress refactors, this is a draft and I did not test is as fully as I wanted, so if you have some time and want to run it locally and try to see if everything works, please do and comment any issues you find!

End-goal: React Router v6

The end-goal is to move the project to React Router v6. But as a first step, we need to migrate to v5 and have it working, then go through the upgrade from "old" v5 to the "new" v5 API, and then upgrade to v6.

• React Router v5 docs
• React Router v5 to v6 upgrade guide — it describes the 2-step migration
• React Router v5 to v6 upgrade using the compatibility package — if we do not want to migrate to the new API first, which I think we do

This PR is only the first step, from v4 to v5 using the same "old" API. It introduces the React Hooks API, so it will be very welcome for the Typescript + Function Component refactor.

Changes

The biggest (and only) change is this.context.router not existing anymore (it switched to the new React hook context).

It has been either removed when not used, or replaced by withRouter + props

In one case, withRouter messed up with ref, so I used a wrapper function component to be able to call useLocation()

To check

Something I did not test yet is the scroll restauration feature (currently using react-router-scroll-4). It will probably need to be updated based on the official docs on the topic.

I am also unsure of the handling of components outside of a router, but I did not know where this feature was used, so could not test it. I check for the existence of this.props.history, but this may not be empty, or withRouter might return an error?

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[Signez]

Something I did not test yet is the scroll restauration feature (currently using react-router-scroll-4). It will probably need to be updated based on the official docs on the topic.

react-router-scroll-4 is compatible with React Router v5 and actually works fine (it's easy to test: create a bunch of posts, then open the home feed — either on single mode or in the navigation column in the advanced mode — then click on a post below the fold of the viewport; when you click on Back, you are not send back to the top!).

It should be replaced in a later PR by <ScrollRestoration> that shares nearly the same API with <ScrollContainer> and is shipped with react-router v6. But for react-router v5, using react-router-scroll-4 seems to be the way to go!

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[zunda]

Unfortunately, this change made it impossible to type posts in Japanese (Firefox 118.0.2 (64-bit)/Mozilla Firefox Snap for Ubuntu running on Xubuntu 22.04, xfwm4-4.16.1-1, fcitx-4.2.9.8-5, fcitx-skk-0.1.4-2). I could only type hiraganas in vowels, i.e., あ い う え お). Typing a consonant, the textarea accepts the single alphabet rather than letting fcitx handle it to compose a hiragana.

Reverting 1b70d7e (along with limiting the Node version to >=16 <21 in package.json for Heroku to install node 20.8.1 rather than 21.0.0) fixed my problem.

[ClearlyClaire]

I'm really confused about how this change could affect input methods, especially in such a convoluted way :/

Interaction between the React app and the input method is possible, with the textarea being a controlled component, but this shouldn't involve routing at all. I'm confused.

[ClearlyClaire]

I have installed fcitx (1:4.2.9.9-1 Debian version) and fcitx-skk (0.1.4-3 Debian version) and cannot reproduce the issue (sorry for the weird overlay in the first half of the video):

Peek.20-10-2023.13-54.mp4

[zunda]

Thanks Claire for looking into this! (So sorry for the typo!) Oddly, I can still reproduce the problem quickly trying the build with this merged (this time, macOS Ventura 13.6 on M1, Safari 17.0, the default Japanese input. I only typed the first w, which should have been converted into 我, once):

Screen.Recording.2023-10-20.at.7.12.03.AM.mov

Below is the build after reverting this PR which doesn't show the first w:

Screen.Recording.2023-10-20.at.7.13.24.AM.mov

I'll further see if this reproduces on another build as well as locally on development environment and report back.

[zunda]

Good news (I'd be able to look into this locally) and bad news (it is still there on my computer) - Mastodon at 1b70d7e without my modifications reproduced the problem with node v18.18.0 and yarn 1.22.15 running on the same Xubuntu I mentioned above in an environment with

export RAILS_ENV=development
export NODE_ENV=development
export NODE_OPTIONS=--openssl-legacy-provider

ng.mp4

At 126cd77, the same keystrokes (kakikukeko) generates the expected text like this:

ok.mp4

I'll continue looking into this but I'm not sure if I'd be able to dig into the React code so any help from anybody will be appreciated!

[zunda]

I saw reports of people having a similar problem typing Japanese on

• Android: https://ukadon.shillest.net/@ponapalt/111280493853443661 and
• iOS: https://don.kumarin.co/@mg/111280520094054136

Editing package.json to downgrade react-router and react-router-dom to 5.1.2 and 5.0.1, the same problem is reproduced on the development environment built and run with:

$ rm -rf node_modules; yarn install && bundle exec rails assets:precompile
$ foreman start -e .env.dev -f Procfile.dev

https://mastodon.zunda.ninja/@zundan/111280758587306196

I'll continue to downgrade other node modules.

[zunda]

It turns out that there is no other node modules I can downgrade from this PR. I'm giving up here and wait for a PR for react-router v6 to land. Sorry for the noise!

[mgmn]

I could see that the problem zunda-san reported is reproduce with textarea in mobile UI, but not so with textarea in the left navigation column in desktop. Perhaps the problem occurs in one (or some) of several compose boxes...

Additionally, access to https://mstdn.example.com/publish directly in desktop (well, this is not normal usage) and you'll seen two compose boxes, then the left one seems correct input but the right one is not.

20231028.mp4

As far as I can see now, the textareas which reproduces the problem would come from same component app/javascript/mastodon/features/compose/index.jsx, but I've no idea what different between other compose forms that would be no problem such as app/javascript/mastodon/features/ui/components/compose_panel.jsx (in left navigation) and app/javascript/mastodon/features/standalone/compose/index.jsx (in sharing form).

[kmycode]

If I did this, it might have worked in my environment.
I know this is not the best solution, but just FYI.

https://github.com/kmycode/mastodon/pull/195/files

[Attention] This is laced with fixes that are unrelated to this issue. For those who came later, please refer to zunda's commit immediately after this one. Again, this is not the best solution.

[zunda]

Thank you @kmycode ! Applying your change on top of main at 4aa05d4, I can type Japanese posts as expected on a server running locally in development environment.

[zunda]

Applying a squashed change zunda@f656108, I can now type Japanese as expected with this and some following PRs merged 🎉. Thank you, again!!

[ClearlyClaire]

I could see that the problem zunda-san reported is reproduce with textarea in mobile UI, but not so with textarea in the left navigation column in desktop. Perhaps the problem occurs in one (or some) of several compose boxes...

Additionally, access to https://mstdn.example.com/publish directly in desktop (well, this is not normal usage) and you'll seen two compose boxes, then the left one seems correct input but the right one is not.
20231028.mp4

As far as I can see now, the textareas which reproduces the problem would come from same component app/javascript/mastodon/features/compose/index.jsx, but I've no idea what different between other compose forms that would be no problem such as app/javascript/mastodon/features/ui/components/compose_panel.jsx (in left navigation) and app/javascript/mastodon/features/standalone/compose/index.jsx (in sharing form).

Thanks for digging into this, I can confirm I can reproduce the issue from the /publish compose form, but not the compose panel one. I can also reproduce the issue with my default input method when pressing dead keys (e.g. ^) as the first character. The input method works fine whenever the textarea is non-empty.

[ClearlyClaire]

I'm still not completely sure what is happening, but this is caused by a spurious mid-compose re-render caused by setState({ lastToken: null }) on the first input. Now, this call to setState should not actually cause a re-render, as the state is unchanged.

However, setState causes shouldComponentUpdate to be called, and that function considers the props, not the state, has changed. Specifically, it considers the children props has changed. This occurs in the /publish mount of the component, but not in the compose panel. I have not managed to understand why yet.

[mgmn]

Yeah this only happens in a first keystroke and no problem thereafter. A workaround mgmn@5059969 that I wrote just for my own server seems working fine without any regressions for now, but I can't describe why this resolves the problem also still wondering what the mechanism causing... Anyway, thanks for your investigation!