Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MBS-13716: Upgrade React to v19 RC #3345

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mwiencek
Copy link
Member

https://react.dev/blog/2024/04/25/react-19
https://react.dev/blog/2024/04/25/react-19-upgrade-guide

It seems that a new version of the react-dom libdefs have not been published to flow-typed yet. I simply moved the existing v18 file and removed all removed functions. I didn't add any of the new functions (we don't use them yet).

--

I'd primarily like to perform this upgrade now in order to use the new react-compiler.

The "current" log files produced by svlogd were completely empty in many cases.
This is because we weren't redirecting stderr to stdout in the service "run"
files.

This only affected services within the musicbrainz-tests container, which
matters because we upload these log files as build artifacts.
(start_template_renderer.sh is also used by the production images, but we don't
use svlogd there.)
Signal propagation didn't work currently where `exec sudo` was used. This meant
issuing a command like `sv down service` wouldn't be able to stop the service.
I reviewed the "Signal handling" section of the sudo manpage, but still didn't
quite understand why this behavior occurs.

The solution is to use `chpst`, which comes with runit and works perfectly
fine. The only change needed is to manually set `$HOME`, which was previously
set via sudo's `-H` flag. (Not every service uses `$HOME`, but I set it
everywhere just to be safe.)
This upgrade caused any test that used `handleAlert` to fail. IIUC, this was
caused by [1], which is an intentional change to no longer show `beforeunload`
alerts at all when the WebDriver is in use.

It's still useful to be able to test that an alert would've been shown, as it's
the only way of verifying that such functionality works to prevent data loss.
I've replaced `handleAlert` with `assertBeforeUnloadAlertWasShown`. The main
difference is that we aren't actually handling the alert (as none are
triggered), but are checking that it would've been shown in a normal browser
environment.

[1] https://chromium.googlesource.com/chromium/src/+/b14c93608871784e41d6d40f1c5952cf24aa39db
For some reason, requests to the redirect service sometimes get stuck ever
since upgrading Chrome, which causes the Selenium tests to hang. Increasing the
number of workers to 2 seems to help.
https://react.dev/blog/2024/04/25/react-19
https://react.dev/blog/2024/04/25/react-19-upgrade-guide

It seems that a new version of the react-dom libdefs have not been published to
flow-typed yet. I simply moved the existing v18 file and removed all removed
functions. I didn't add any of the new functions (we don't use them yet).

A strange failure occurred in External_Links_Editor.json5: when typing
"https://www.discogs.com/David-Bowie--Blackstar/release/7949394", it somehow
omits the "s" in "release", which causes a validation error. I have no idea how
the React upgrade could have caused this, and I can't reproduce it outside of
the tests (e.g., by pasting the same URL manually, or by typing it slowly). The
only way around this I could find was avoiding `sendKeys` in the `type`
command, so I'm just using `setInputValueForReact` again (which is already used
for clearing the input).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant