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

1541 update node migrate cra to vite #3

Merged
merged 16 commits into from
Jun 17, 2024

Conversation

sinejespersen
Copy link

@sinejespersen sinejespersen commented Jun 4, 2024

Link to ticket

https://leantime.itkdev.dk/#/tickets/showTicket/1569
https://leantime.itkdev.dk/#/tickets/showTicket/1568

Description

As it seems CRA is no longer being maintained by fb, I am migrating to Vite.

  • upgrade node from 14 -> 20
  • yarn upgrade
  • remove unused dependencies (test-stuff)
  • move from CRA/react-app-rewired -> Vite
  • Go from Winston logger to Pino logger
    • I tried choose Pino, because it works and because it has a MIT license

Screenshot of the result

image

It works!

If your code does not pass all the requirements on the checklist you have to add a comment explaining why this change
should be exempt from the list.

Additional comments or questions

  • figure out what to do with the winston-logging, I cannot seem to make it work, and it seems from Vites github page that they do not intent to make a fix that makes it easy.

As stated above, Pino will be replacing Winston.

@sinejespersen sinejespersen changed the title Feature/1541 update node migrate cra to vite Feature/1541 update node migrate cra to vite (WIP) Jun 7, 2024
@sinejespersen sinejespersen marked this pull request as draft June 7, 2024 08:43
@sinejespersen sinejespersen changed the title Feature/1541 update node migrate cra to vite (WIP) 1541 update node migrate cra to vite Jun 7, 2024
@sinejespersen sinejespersen marked this pull request as ready for review June 7, 2024 11:15
@sinejespersen sinejespersen requested a review from tuj June 7, 2024 11:15
package.json Outdated Show resolved Hide resolved
@@ -25,7 +25,7 @@ const ConfigLoader = {
) {
resolve(configData);
} else {
fetch(`/client/config.json?ts=${nowTimestamp}`)
fetch(`config.json?ts=${nowTimestamp}`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to test how this works on the server...

@@ -4,7 +4,7 @@
export default class ReleaseLoader {
static async loadConfig() {
const nowTimestamp = new Date().getTime();
return fetch(`/client/release.json?ts=${nowTimestamp}`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also requires a test in server setup.

src/touch-region.jsx Show resolved Hide resolved
@sinejespersen sinejespersen requested a review from tuj June 10, 2024 11:13
@sinejespersen sinejespersen merged commit f5e3255 into develop Jun 17, 2024
4 checks passed
@sinejespersen sinejespersen deleted the feature/1541-update-node-migrate-CRA-to-vite branch June 17, 2024 07:30
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.

2 participants