spring-petclinic-reactjs icon indicating copy to clipboard operation
spring-petclinic-reactjs copied to clipboard

Convert the whole repository to pure frontend Reactjs project

Open firasrg opened this issue 2 years ago • 43 comments

Problem

I'm unable to run the app locally, I see many errors ❌ The app is built with Spring Boot v1.4 and I'm with JDK 17?! It seems this project has not received updates for long time (Last commit on /src folder was 6 years ago!), this indicates that it has become outdated so it lacks compatibility with newer techs, patterns and best practices.

Solution

I suggest to do an upgrade (ideally to latest version Spring Boot v3).

FYI: The main Spring PetClinic project is actively maintained and regularly updated! It can serve as an excellent reference to guide the upgrade process here.

Please let me know if you have any questions or require further information to support this issue.

firasrg avatar Jun 10 '23 17:06 firasrg

FYI, I've started working on this issue in a forked repo.

firasrg avatar Jun 10 '23 23:06 firasrg

hello, I agree that it will be good to do an upgrade. I wanted to install it today and it took me a while to figure out that we need Java 8 to run it because of the javax.xml.Binding packages incompatibility.

@firasrg I can see that you have started working on it. Happy to help if needed.

michaelisvy avatar Jun 11 '23 09:06 michaelisvy

Hi @firasrg Thanks for your proposition. We are looking contributors to upgrade all the forks of the Petclinic Community and you are welcome. I think you can indeed draw inspiration from the main repository. I let you submit a Pull Request when you are ready.

arey avatar Jun 11 '23 15:06 arey

Thank you, @firasrg for your work!

@all: I wonder if we should follow the spring-petclinic-angular example and drop the backend part here and only include the react frontend.

Pro:

  • we can focus on implementing the React frontend (I don't think in the backend should be any React-specific stuff)
  • people can compare angular/react solutions without having to worry about the backend

Con:

  • we need to make sure that we're compatible with spring-petclinic-rest. (I think this is not really a problem as we can pin a commit or tag here).

What do you think?

nilshartmann avatar Jun 11 '23 18:06 nilshartmann

@nilshartmann Hmm if there is already a reliable backend app, why repeating it here ? In that case I sugges to make this repo only for frontend app.

Ultimately, if absolutely you need backend and frontend source files together, then you can use Git Submodules feature, but that will require a new repo (I think).

So project hierarchy becomes like this

  • Spring Pet Clinic (main repo)
    • React App (repo frontend)
    • Rest API App (repo backend)

Otherwise I agree to make it fully frontend like the one for Angular.

firasrg avatar Jun 11 '23 19:06 firasrg

@firasrg: Git Submodules sounds like a good idea.

I think we don't need a new repo:

  • Spring Pet Clinic (this repo)
    • frontend/ (this repo)
    • REST API app (repo backend via submodule)

nilshartmann avatar Jun 11 '23 19:06 nilshartmann

Hmm, that would be nice! So are you (collaborators/maintainers) going to do this radical change or contributors?

firasrg avatar Jun 11 '23 21:06 firasrg

Hello Nils and Firas. If we could use the existing spring-petclinic-rest as backend it coud be great. The spring-petclinic-reactive demonstrates how to use a git submodule.

arey avatar Jun 12 '23 07:06 arey

Hi all,

again thanks a lot for your comments and engagement! 👍 😊

I would suggest:

  • we're using only one issue to track progress for backend and frontend, as both "parts" are connected
    • We can use this one and close #25 instead
  • we can create a new branch for the work
    • when the work is done, we can also rename master to main
  • We have to somehow split the work so that @michaelisvy and @firasrg can work together in parallel (and whoever else want to join)
    • I currently don't have that much time but I'm more than willing to help if you have problems, questions or need any kind of review
    • As an "inspiration" for the React client, you could also have a look at my GraphQL React example. Not sure if that helps much, because it's build using create-react-app and also of course with GraphQL API, but maybe you can "steal" some components or ideas :-)

What do you think?

nilshartmann avatar Jun 12 '23 07:06 nilshartmann

Hi again @nilshartmann And thank you for this feedback!

The other issue is closed🚪.

I think that tasks like rename main branch and adding submodules can be done by someone having authorities unlike me or any ordinary contributors.

As for the upgrade I think i can do i it effectively!

@michaelisvy I don't think that splitting the upgrade task is a good idea as the current version looks simple. Maybe we can add a 1st new feature, like a user authentication.

What do you think?

firasrg avatar Jun 12 '23 12:06 firasrg

I have another interesting suggestion (it can be in another issue if you like it).

Make the client app as a React Native project, this can bring more interested people to the field 🌝.

As React Native gives possibility to build native apps (mobile/tablet) for Android/iOS in addition to Web with one codebase!

there is a nice framework called Expo that takes care of all native files. Im familiar with Expo and use it frequently to develop React Native. With Expo you only use JS/TS and Json files, no gradle files, no Xml, no swift, no Java. Expo is similar to Vite and CRA, it opens the road to work quickly and focus on logic rather than configurations.

Mobile apps now are trending.. many customers today, search to build more than one product with 1 codebase (with less efforts qnd time)

What do you think?

firasrg avatar Jun 12 '23 12:06 firasrg

Hi @firasrg,

while I think this is a good idea, I think that would make another/different spring-petclinic example project.

I'd like to go here with React, so people can compare with the angular approach, can get an idea how a React codebase would look like and so on.

nilshartmann avatar Jun 12 '23 14:06 nilshartmann

Hi @firasrg

We don't yet have a mobile version of Petclinic dedicated to smartphones. The Kotlin version is a classic webapp, not an Android application. Having a fork of Petclinic as a native application is a great idea. Why not with React Native? But like Niels, I'd be in favor of creating another repository. Let's perhaps concentrate first on upgrading this repo.

arey avatar Jun 13 '23 06:06 arey

hello, I started something here: https://github.com/michaelisvy/petclinic-react It's my first React application, so it's not great and still work in progress.

michaelisvy avatar Jun 13 '23 14:06 michaelisvy

Hello everyone!

I did some good refactoring (still in progress). You can review it here 👈 .

Tbh, it's not like I expected at the beginning, it needs some hard work, as long as we think quality and long term maintenance.

@michaelisvy, thank you for this feedback, would you like to take a look at my version ? 😁

I'll keep you updated, see you

firasrg avatar Jun 13 '23 22:06 firasrg

yes it definitely takes a while. I'm happy to contribute to your repo if you think we can share the load. I saw that you use Bootstrap. Is it more popular than Material-UI?

michaelisvy avatar Jun 14 '23 10:06 michaelisvy

Im using bootstrap because it's mandatory to render styles correctly like in the old version.

I think ti go step by step: first I make it show up as expected then we see what to change.

Alright, Im going to add you as a collaborator

firasrg avatar Jun 14 '23 10:06 firasrg

thanks @firasrg ! I see you've added me as a contributor. Is there something I can do in priority?

michaelisvy avatar Jun 16 '23 00:06 michaelisvy

You can open an issue and work on it 🤷🏻‍♂️

firasrg avatar Jun 16 '23 06:06 firasrg

ok I've sent you a pull request. Still a lot to do :)

michaelisvy avatar Jun 17 '23 07:06 michaelisvy

@michaelisvy thank you for your help.

Btw it seems your PR is sent to the upstream rep not the forked repository, the one I created.

👉 I think it's too early to do that, as you said, there is lot todo. So i invite you to change the merge target to the main branch in the forked repo only. So i can review , merge or reject.

firasrg avatar Jun 17 '23 08:06 firasrg

ah indeed! I've closed the previous PR and created another one here: https://github.com/firasrg/spring-petclinic-reactjs/pull/4

michaelisvy avatar Jun 17 '23 09:06 michaelisvy

hello, would it be possible to open our code base to more reviewers? For instance, @shonoru has reviewed my pull request but I don't think he has the right to approve it.

michaelisvy avatar Jul 10 '23 14:07 michaelisvy

Hello @michaelisvy !

Thank you for your feedback and activities! I value all your efforts.

I think you can do merges by yourself as you are a member of that forked repo. I trust you!

Excuse me for being inactive during all this time, i have priorities..

Feel free to contact me, if you like, on my email, discord or any.. we can talk together and come to an idea

firasrg avatar Jul 10 '23 14:07 firasrg

ah is it? I thought you had to approve. I'll try to merge it. Thanks for your feedback on my Pull Requests, I really learned from it :).

michaelisvy avatar Jul 10 '23 14:07 michaelisvy

actually, I don't seem to be able to merge. I can see this message: "Merging is blocked Merging can be performed automatically with 1 approving review." Maybe it's a setting you can change?

michaelisvy avatar Jul 10 '23 15:07 michaelisvy

Retry now please @michaelisvy

firasrg avatar Jul 10 '23 15:07 firasrg

don't seem to be able to merge

Please see, https://github.com/firasrg/spring-petclinic-reactjs/pull/8#issuecomment-1629187645

shonoru avatar Jul 10 '23 15:07 shonoru

it seems the merge didn't work because you have a review comment still open! Remeber to resolve them before thinking to merge next time.

I've made merge rn, you can notice it. Also, in order to get free from dependency on me, you can fork my repo, and continue your progress then do pull request. As for my repo, i still interested in it, just not finidin enough time.

Thanks all

firasrg avatar Jul 10 '23 15:07 firasrg

thanks @firasrg , I will fork your repository then.

michaelisvy avatar Jul 10 '23 22:07 michaelisvy