Migrate react-scripts to vite #696

mfechner · 2024-10-15T06:45:01Z

Fixes #695

To test this modification:

(cd ui && yarn)
(cd ui && yarn build)
go run .
(cd ui && yarn start)

Now access the address yarn start shows you.

mfechner · 2024-10-17T04:20:19Z

Ok, to sum it up.
The migration to vite is more or less done.
But it seems that state management is broken, after a login the message component is not loaded.

I tried to update the router, but that is even worse. They broke the complete API and do not even provide a migration script for it.

As I mainly develop in Angular I need to read about react before I continue, but it seems that this upgrade to vite is depending on updating all libs to a more current version.

If someone else here have a strategy how to update the ui, any feedback is welcome.
I understand now the comment from @jmattheis that this will not be an easy task ;)

jmattheis · 2024-10-18T19:42:33Z

@mfechner I feel you (:, I think the only good way it to start from scratch and copy component by component into a new project, so that you don't have to fix 100 errors to get something working. It's on my todo list, sadly it's multi hour effort.

mfechner · 2024-10-22T15:36:45Z

@jmattheis thanks a lot for your feedback. I currently study react to better understand react itself.
It's to early to really make a statement here, but why you do not use the standard state management of react. That would help to get rid of a third party lib and would also simplify upgrade in the future.

I'm on this task but it will take some time ;)

jmattheis · 2024-10-25T09:35:39Z

The codebase is quite old, pre react hooks. In my experience it wasn't that ergonomic to use contexts for global state, so I've used a lib. Currently I like https://github.com/pmndrs/zustand for it's simplicity and small footprint.

mfechner · 2024-12-28T19:11:20Z

I started now with a rewrite, but this will be a massiv change.
Steps I plan to do:

rewrite all class based components as functional components to be able to use all hook
upgrade react in first step to 18.3.1 and later to 19
upgrade material to v6
replace state management with @reduxjs/toolkit and react-redux
upgrade react-router to 7
use vite 6

I think it will be nearly impossible to get this changes sliced into small commit, as all steps link somehow together.
Maybe later try to get also rid of not needed third party libs like axios. The fetch API is normally more than enough nowadays.

I update you, if I have something to show, but it will for sure take some days maybe some weeks.

In DEV mode, you just do: go run . (cd ui && yarn start) In dev mode there is a proxy in vite used to not have CORS problem. You can access the UI on: http://localhost:5173 The UI prefix every request to the backup with `/api` so the proxy can correctly handle it, the proxy strips the `/api` so the backend is equal to production mode.

…n FreeBSD

…ket working

changing (observed) observable values without using an action

mfechner · 2025-01-01T12:57:19Z

This is now a first version we can start.
The newly rewritten react app looks ok and tests I did are working fine.
There can be some finetuning, like loading indicator or use redux RTK query.

There are some todos left (infinite loading component for messages), but it is now a good starting point.

The only problem I have is that puppeteer sometimes is just not loading the page and therefore tests are not running.
Maybe someone has an idea why?

@jmattheis would be nice if you can have a look, it was really a lot of work

mfechner · 2025-01-02T08:25:48Z

All tests are now working fine for me.

jmattheis · 2025-01-02T17:10:53Z

I'll have a look at this in the coming days, but I'll take some time.

mfechner · 2025-01-02T18:10:43Z

I'll have a look at this in the coming days, but I'll take some time.

thank you very much. I found other small issues I corrected in the meantime.
The webinterface itself seems to work, but I have problem with the tests.
Sometimes they are failing (about 2 failures if doing 10 test runs).

The problems I found is that for most actions a submit or a click on a button was not blocking (running in background), which I reverted to the behaviour the released gotify version has.
But something with the test is maybe broken and as I never used pupeteer before, that is hard for me to analyse, so I need some help with the tests.

Later we can addnice indicators, if you e.g. click on Submit the button is replaced by an loading indicator.

I have here some very nice ideas, but I will provide this later in another PR, as that is not related to this topic.

Let us first get the update to react 19 merged, then we can continue further in smaller steps.

Maybe we can release these changes as a beta version to enable users testing it.

jmattheis

Not finished with reviewing, this was only glancing through some files.

ui/src/App.tsx

jmattheis · 2025-01-05T16:42:03Z

ui/serve.go

@@ -32,7 +31,7 @@ func Register(r *gin.Engine, version model.VersionInfo, register bool) {
 		return strings.Replace(content, "%CONFIG%", string(uiConfigBytes), 1)
 	}

-	ui := r.Group("/", gzip.Gzip(gzip.DefaultCompression))


I cannot reproduce the net:ERR_INCOMPLETE_CHUNKED_ENCODING error with your listed steps:

cd ui yarn build yarn run test user

with having the compression enabled here.

If a test is failing, I always see the following error message in the javascript of the puppeteer browser:

or

Not sure if that is a problem related to my machine, some virus scanner or anything else.

But if I remove this commit again, all tests are failing 100% with the error message:

ui/vite.config.ts

ui/src/message/Messages.tsx

jmattheis · 2025-01-05T19:29:20Z

ui/src/message/Messages.tsx

+                <LoadingSpinner />
+            ) : hasMessages ? (
+                <div style={{width: '100%'}} id="messages">
+                    {/* TODO: maybe replace ReactInfitite with react-window, which is also documented here: https://mui.com/material-ui/react-list/#virtualized-list */}


I've recently tried a lot of window libraries for another project and landed on https://tanstack.com/virtual/latest/docs/introduction react-window doesn't play to well with having a window over the whole screen.

ui/src/common/SettingsDialog.tsx

jmattheis · 2025-01-05T19:39:52Z

ui/src/store/ui-slice.ts

+}
+
+const initialUiState: UiState = {
+    themeKey: 'dark',


I'm not sure I like this too, previously this was local component state, because it was only used in two places, and passed downwards to one component. Sure adding this to the global state can be done, but local component state is better as it's simpler and easier to navigate.

These refactorings making this review much more effort as it's not just library change / upgrades, but also logic changes which aren't 100% necessary.

I had a lot of problems in the beginning to understand which prop is passed to which component and why
Using a global state reduces the need of props drilling which is making the componets match leaner and easier to read.

But if you prefer a global state in e.g. app.tsx, I can remove these UI related states from the global state and move it to app.tsx and pass them via props drilling to the components.
But to be honest it will be technically equal and will just make the components harder to read.

But as you prefer.

I prefer it to be as close as possible to the original implementation for now, as it makes less changes to review and less possibility of bugs, because the code already runs for some time. This includes the moving of the selected plugin state into redux.

ok, I will move the state/slice/action definition to the component.
But I will not remove the ui state management and will change everything back to prop drilling.
This is error prone and we will for sure just add new bugs by this. The UI state is working perfectly fine.

This is error prone and we will for sure just add new bugs by this.

How is it error prone to pass properties to sub components? TypeScript is used, so it's not like you can miss passing the props?

Rewrite all components to functional component using hooks Upgrade react-router to 7.1.0 Use reduxjs/toolkit and react-redux for state management Upgrade material to 6.3.0 Updated most dependencies to current versions

This caused a failure correctly forwarding the user to the login page

…o missing network

Replace react-codemirror2 which is unmaintained by @uiw/react-codemirror and applied required API changes to ReactMarkdown

… the store more than once

…nSubmit handler

…container and can be accessed for the windows host. This is required to use/test plugins with gotify

while working with the production bundled application.

…ion number)

…ld not happen, as the key on already rendered table does not change)

…on bundle

We hold now all messages in the state. If message of one app are refresh, make sure to sort them by date into the existing messages

Axios does not need this for error handling

…f the matching component

Except for the delete actions, as the test does not expect it there

mfechner · 2025-01-08T19:32:33Z

Failing tests seem to be related to puppeteer upgrade.
I downgraded it and I see the tests are working better.

I will work through the commits again and will maybe rebase some of them, so they match better.

@jmattheis the current implementation does not have the virtual scrolling for messages, should I include it here, ot as another PR?
Also paging is currently not implemented (which I can currently not test, so I have to create a datasource to work with paging).

mfechner mentioned this pull request Oct 15, 2024

FreeBSD builds #199

Open

mfechner force-pushed the react_scripts_to_vite branch from f296d85 to 1bf4902 Compare January 1, 2025 12:44

mfechner added 9 commits January 1, 2025 14:44

Use plugin-react instead of plugin-react-swc, as that will not work o…

289f100

…n FreeBSD

Small fixes on state management and vite configuration to make websoc…

96d09ef

…ket working

Started to fix issues with state management (mobx)

a488460

changing (observed) observable values without using an action

Generate a build/asset-manifest.json

45821b4

Fix problem in change password and some other state management problems

7a778b5

Clean up changes to be consistent

1ec8b23

Make it possible to run tests with vitest

1200f7c

Define global as it is required by react-codemirror2

560e3d9

mfechner force-pushed the react_scripts_to_vite branch from 1bf4902 to 3d6fca9 Compare January 1, 2025 12:50

mfechner marked this pull request as ready for review January 2, 2025 08:25

mfechner requested a review from a team as a code owner January 2, 2025 08:25

jmattheis self-requested a review January 2, 2025 17:11

jmattheis reviewed Jan 5, 2025

View reviewed changes

mfechner added 5 commits January 8, 2025 14:12

Upgrade react to 18.3.1

122ef6a

Rewrite all components to functional component using hooks Upgrade react-router to 7.1.0 Use reduxjs/toolkit and react-redux for state management Upgrade material to 6.3.0 Updated most dependencies to current versions

Fix rimraf api changes

1a5ca1f

Update all packages to current version including react 19

8596883

Fix problem with failed auto-login

df5dd66

This caused a failure correctly forwarding the user to the login page

Fixed change password and logout in e2e tests

d36a476

mfechner force-pushed the react_scripts_to_vite branch from 6cd4b90 to cf08815 Compare January 8, 2025 12:18

mfechner added 27 commits January 8, 2025 14:18

Fixed last failing tests for messages

ad3e80a

Fix automatic logout if client was removed

5967dfc

Do not load an image if it is null or undefined

221d286

Fixed wrong display of messages (filter of selected App was not applied)

e388b39

Display a snack message if a single message was deleted

3b30fd6

Hide the loading indicator if the fetch of messages fails, e.g. due t…

09f9acf

…o missing network

Finished rewrite of the Plugin details view to a functional component

f1ec87f

Replace react-codemirror2 which is unmaintained by @uiw/react-codemirror and applied required API changes to ReactMarkdown

Make sure, the websocket is a singleton that message are not added to…

82b22a7

… the store more than once

Fixed Register user feature and use react 19 action instead of form o…

019ae7e

…nSubmit handler

Removed unused code

540ac94

let vite listen not only on localhost so it can be started in a WSL2 …

7de2f60

…container and can be accessed for the windows host. This is required to use/test plugins with gotify

Fixed direct loading of sub-pages like /#/messages/3

c299cb6

while working with the production bundled application.

Make sure new Material uses some colors like the old material theme did

c2cb751

Fixed layout problem caused from Material upgrade

c0c2442

Fixed passing of build related information to the react ui (like vers…

a6422c9

…ion number)

Removed not need options due to vite usage

4966d85

Replace the application that it does not change the order (which shou…

61ed726

…ld not happen, as the key on already rendered table does not change)

Do not inline sourcemaps anymore to reduce the file size for producti…

6683377

…on bundle

Remove TODO, we keep the name of the component like it is

2741ec5

Use the same url while working in dev mode compared to production mode

3c174ff

Fixed loading and refreshing of messages

e266a8d

We hold now all messages in the state. If message of one app are refresh, make sure to sort them by date into the existing messages

Fix typo in test

7478bf6

Reenable React Strict Mode

32c67ab

Remove the async sendRequest method

fec8a01

Axios does not need this for error handling

Moved state/action definition from directory store into directory o…

4453883

…f the matching component

Remove obsolete files

eae7a4c

Added loading indicators

92a9608

Except for the delete actions, as the test does not expect it there

mfechner force-pushed the react_scripts_to_vite branch from cf08815 to 92a9608 Compare January 8, 2025 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate react-scripts to vite #696

Migrate react-scripts to vite #696

mfechner commented Oct 15, 2024

mfechner commented Oct 17, 2024

jmattheis commented Oct 18, 2024

mfechner commented Oct 22, 2024

jmattheis commented Oct 25, 2024

mfechner commented Dec 28, 2024

mfechner commented Jan 1, 2025

mfechner commented Jan 2, 2025

jmattheis commented Jan 2, 2025

mfechner commented Jan 2, 2025

jmattheis left a comment

jmattheis Jan 5, 2025

mfechner Jan 7, 2025

jmattheis Jan 5, 2025

jmattheis Jan 5, 2025

mfechner Jan 6, 2025

jmattheis Jan 6, 2025

mfechner Jan 7, 2025

jmattheis Jan 7, 2025 •

edited

Loading

mfechner commented Jan 8, 2025

Migrate react-scripts to vite #696

Are you sure you want to change the base?

Migrate react-scripts to vite #696

Conversation

mfechner commented Oct 15, 2024

mfechner commented Oct 17, 2024

jmattheis commented Oct 18, 2024

mfechner commented Oct 22, 2024

jmattheis commented Oct 25, 2024

mfechner commented Dec 28, 2024

mfechner commented Jan 1, 2025

mfechner commented Jan 2, 2025

jmattheis commented Jan 2, 2025

mfechner commented Jan 2, 2025

jmattheis left a comment

Choose a reason for hiding this comment

jmattheis Jan 5, 2025

Choose a reason for hiding this comment

mfechner Jan 7, 2025

Choose a reason for hiding this comment

jmattheis Jan 5, 2025

Choose a reason for hiding this comment

jmattheis Jan 5, 2025

Choose a reason for hiding this comment

mfechner Jan 6, 2025

Choose a reason for hiding this comment

jmattheis Jan 6, 2025

Choose a reason for hiding this comment

mfechner Jan 7, 2025

Choose a reason for hiding this comment

jmattheis Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

mfechner commented Jan 8, 2025

jmattheis Jan 7, 2025 •

edited

Loading