bastislack / highline-freestyle

Webapp for Highline Freestyle
GNU General Public License v3.0
10 stars 9 forks source link

Rewrite app with TypeScript and Vite #251

Closed WaldemarLehner closed 1 year ago

WaldemarLehner commented 1 year ago

This is not complete as of now, but I figured I'd set up a draft so you can see what would change and ask questions if something looks off to you.

Closes #245 Closes #242 Indirectly closes #219

Below are some notes I made during the migration:

About

This Markdown defines notes that came up during the Migration. They should be resolved during or after the migration and deleted afterwards.

Localization

I have disabled localization for now as there were issues with Babel. Could look into it after the Migration and re-enable. Another question is as to if an i18n-Provider is even needed with the very low usage of translated texts.

Another question I have is what benefits lingui has to offer. Is there any reason it is being used to the (more common) i18next?

Routing

react-router added "Data Routers" in v6+. It would be beneficial to switch a data router in the future.

Data Routers provide the same functionality as the SSR Rendering Framework "Remix" by the same developers. You get nested routes with an isolated data flow. I could go into greater detail about this in a GH Issue.

Dependencies

Some dependencies are out of date.

Material UI

Fab from @material-ui/core comes to mind here. It looks like @material-ui/core got renamed to @mui/material, but now also depends on the emotion Styling Cache.

Is the FAB from MUI really needed? I don't see much merit in using it as the FAB is the only time it is used. Also… if MUI is preferred, we should get rid of Bootstrap. No need to have two Styling Frameworks, really.

rc-slider

Same as with MUI. Is a separate Library needed for that? Doesnt Bootstrap or MUI already provide that functionality?

Database

The only significant change I did during the Migration was to extract Migrations into a separate Module and also change from Papaparse importing from a JS-Module to using the virtual module provided by a new Vite Plugin.

The Database needs other changes, which I have chosen to not add for now. The PR will be bloated enough already.

bastislack commented 1 year ago

Here are some comments regarding your notes:

Localization

We haven't merged the translations yet to the develop branch (see #198) but we plan to translate every part of the app at least to spanish for now. At a later stage also french translations are planned. So I guess an i18n-provider makes sense, right?

Regarding the choice of LinguiJS, maybe @rorystephenson has some comments about this, as he was the one to choose it :)

Routing

I am not experienced with this so feel free to open an issue and explain the problem a bit more in depth :)

Dependencies

What do you mean with: 'Some dependencies are out of data' ?

Regarding MUI this is basically a question whether we choose MUI or Bootstrap, right? Also here I don't have much experience with this and I guess it would be best to choose the one that more of us have experience with?

Regarding the slider: I was actually not aware that there is a slider in Bootstrap. But after a quick check I couldn't find an easy solution for handling the two bullets in the range, apart from this: Multi-Range.

WaldemarLehner commented 1 year ago

What do you mean with: 'Some dependencies are out of data' ?

Oh. That's a typo. It was supposed to be "out of date" :D

Regarding the slider: I was actually not aware that there is a slider in Bootstrap. But after a quick check I couldn't find an easy solution for handling the two bullets in the range, apart from this: Multi-Range.

Here is also a solution with MUI https://mui.com/material-ui/react-slider/#range-slider

I am not experienced with this so feel free to open an issue and explain the problem a bit more in depth :)

I will do once the migration is done :)

bastislack commented 1 year ago

Here is also a solution with MUI Cool, it looks super clean :) I'll make an issue where we can discuss Bootstrap vs. MUI

rorystephenson commented 1 year ago

Regarding the choice of LinguiJS, maybe @rorystephenson has some comments about this, as he was the one to choose it

Hey sorry on the slow response. As far as I can remember I think it I choose it because it looked simple (sometimes localisation libraries are monsters) and had a handy way of being able to generate the (empty) localisations files based on declarations in the code.

That said if it's causing issues that can't be resolved then it's definitely worth considering alternatives.

I'd avoid dropping localisation if possible as it gets harder and harder to add it in the later it is done.

Dosbodoke commented 1 year ago

Let me know when this PR is ready for Review. It will improve the developer's performance so much that I think we can get rid of all the issues on the backlog.

WaldemarLehner commented 1 year ago

It's a long Way :D

I am currently waiting for #239 and #252 to be resolved / decided upon.

On Fri, 14 Apr 2023, 14:16 Juan Andrade @.***> wrote:

Let me know when this PR is ready for Review. It will improve the developer's performance so much that I think we can get rid of all the issues on the backlog.

— Reply to this email directly, view it on GitHub https://github.com/bastislack/highline-freestyle/pull/251#issuecomment-1508415589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGTHPAMQYNYA2WE253JNEHDXBE527ANCNFSM6AAAAAAWIHMONA . You are receiving this because you authored the thread.Message ID: @.***>

Dosbodoke commented 1 year ago

I found this library for Airbnb, it can help do the job.

https://medium.com/@shaitu/how-to-migrate-from-javascript-to-typescript-using-the-ts-migrate-tool-c7936b676c99

WaldemarLehner commented 1 year ago

Most of the automatable stuff is done. It's mostly just stuff where weird reassignments happen or variables take up multiple shapes.

Dosbodoke commented 1 year ago

I just finished a personal project for a Highline Festival, it was amazing. I developed it with Next and TypeScript + Supabase for the backend and it was so fun to develop it. Click here if you want to take a look

When I was contributing to this codebase, I found it so difficult because of the lack of TS. I think that this issue will boost a lot the project, after my last comment I did not see any evolution. Can I help with this task, if yes, what can I do?

WaldemarLehner commented 1 year ago

Hey @Dosbodoke, I am currently waiting for #239 to get resolved. My Idea would be to make a solid, well designed Data Layer, which the UI Layer would interact with.

From what I remember there were also talks about rewriting the UI (#262), which would go well with a complete rewrite.

rorystephenson commented 1 year ago

TypeScript would definitely be a big improvement. I think nobody has had the time to complete that migration yet. I'm unfortunately busy with other projects right now.

WaldemarLehner commented 1 year ago

App is being rewritten from the ground up -- which makes this PR obsolete

Dosbodoke commented 1 year ago

App is being rewritten from the ground up -- which makes this PR obsolete

Is there a related issue, or a new Repo? I Would love to help with this new phase.

From what I have experienced here, there are two key points that would bump the project to the next level, those are:

WaldemarLehner commented 1 year ago

Is there a related issue, or a new Repo? I Would love to help with this new phase.

Rewrite is happening in this repo on the rewrite and rewrite-* Branches.

Much of what has been discussed for the rewrite can be found in #239 #245 #252

From what I have experienced here, there are two key points that would bump the project to the next level, those are:

* TypeScript

New Stack makes use of Vite, TypeScript, Vue 3, Zod, TailwindCSS, etc …

* A real database

We decided to stick with Dexie.js as it is the closest thing we can get to a "real" database while staying in the browser without the need to host a backend. It also allows to access the App while offline.

bastislack commented 1 year ago

@Dosbodoke If you want you can join our discord channel, just tell me your name on Discord :)

Dosbodoke commented 1 year ago

@Dosbodoke If you want you can join our discord channel, just tell me your name on Discord :)

I would love to.