-
Notifications
You must be signed in to change notification settings - Fork 44
Replace Create React App with Vite #1335
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
Conversation
500780b
to
9766223
Compare
The three-click centring E2E test still seems to be failing after mounting the sample (the "Current" tab doesn't change to "Sample: test - test"). I have a feeling it's linked to the fact that socket.io switches to I've skipped it for now, since it seems to work outside of Cypress. |
}); | ||
|
||
it('Each click is rotating the sample by 90 degrees', () => { | ||
it.skip('Each click is rotating the sample by 90 degrees', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping this test for now, as explained, but if you have any clue @walesch-yan, please let me know 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have no direct answer to this behavior. My guess would be that some configuration mismatch on cypress' side leads to networking issues. This definitely needs to be investigated in more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always give thumbs up for cleaning up, removing things, and modernizing : )
Very nice thanks for the excellent work :) I don't mind using a browser extension for the redux store debug. I guess its possible to add a variable for it if its absolutely necessary, @meguiraun and @elmjag ? Maybe in that case it would be nice to have the option to have no logging at all. But a question out of curiosity, how do you do to update browser version and so on if you don't have access to internet ? I'm happy to merge this :), there is a conflict from a previously merged PR, maybe you could have a look @axelboc |
As far as I understand we have our own proxies/mirrors for the system package repositories of the Linux distribution. |
I see, I'll go a head and merge this later today and we can sort out any remaining issues with how to handle the redux logging in a future PR, let me know if you think this approach is too progressive or if you have other concerns. |
I've added an environment variable called |
Thanks ! |
I am thinking maybe this needs some documentation... If things have changed for developer experience in a non-backwards-compatible way -- for example the change of port number, removal of redux-logger (assuming we go through with this part) -- then it might be worth an announcement on the mailing list. And then it might be worth documenting (in |
@fabcor-maxiv: The last commit also added the possibility to use The missing part is perhaps the port change from 3000 to 5173 ? |
Oh, good, I had not seen this had been added : )
|
Environment variables are read when the dev server starts ( |
I can revert the port change if it's easier? |
Good idea, I can add a section about the devtools. |
I'd rather we leave things to their default values. But maybe some people will be surprised by the change. That is why I think an announcement on the dev mailing list would make sense. And/or there could be a line in our documentation explaining how to set the port to 3000. But I assume it would be no more than a copy-paste of Vite's documentation and a link back to Vite's documentation, which I find slightly pointless to have (and maintain) in our own documentation pages. So that would not be my preferred solution. Independent from all that, can we have a paragraph in https://mxcubeweb.readthedocs.io/en/latest/dev/front-end.html that says that we have Vite as the builder (with link to their home page)? |
Yep, working on it 👍 |
I took the opportunity to expand the front-end documentation: https://github.com/mxcube/mxcubeweb/pull/1335/files#diff-425da2324e83ab4adea4825089d26a6b2b0d0dc15cbd54a452499c1f4445909f |
Nice, also good with the best practices for functional components and hooks connecting to the store 👍 |
Fix #1273
See Vite's documentation to learn about this modern front-end bundler.
I've kept the build folder the same (
ui/build
) but I did change the front-end port to5173
, which is Vite's default port. You'll note that both the dev and build are much, much faster thanks to esbuild — the "UI" CI workflow is more than one minute faster.