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

Remove redundant code [Refactoring] #135

Open
shravan20 opened this issue Apr 30, 2021 · 8 comments
Open

Remove redundant code [Refactoring] #135

shravan20 opened this issue Apr 30, 2021 · 8 comments
Assignees
Labels
documentation Improvements or additions to documentation duplicate This issue or pull request already exists enhancement New feature or request good first issue Good for newcomers hacktoberfest hacktoberfest-accepted help wanted Extra attention is needed Little work This task takes little time to complete. (ETA couple of days) refactoring The changes in code without affecting functionality to improve the code

Comments

@shravan20
Copy link
Owner

Is your feature request related to a problem? Please describe.
Constant Values stored as JSON values are duplicated across the frontend and backend version.

Describe the solution you'd like
Remove the duplicated version inside the /frontend/src/util/animation/index.js file and use the existing backend-version src/animations/animation.js file

@shravan20 shravan20 added documentation Improvements or additions to documentation duplicate This issue or pull request already exists enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers Little work This task takes little time to complete. (ETA couple of days) refactoring The changes in code without affecting functionality to improve the code labels Apr 30, 2021
@shravan20 shravan20 added this to the Release of V2.0 milestone Apr 30, 2021
@raj5036
Copy link

raj5036 commented Aug 27, 2021

Hi @shravan20 , I was studying the codebase, and wanted to start contributing.

This issue #135 seems like a good start. My question is should I delete the duplicate file altogether or just use the existing backend version while keeping the duplicate.

Thanks & Regards,
Raj Karmakar

@VishalSharma2000
Copy link

Hello @shravan20, I was walking through the code and as you have mentioned I found that the animation file is repeated in the frontend as well as backend. I think we can remove this redundant code in the below two ways

  1. As you have told, we can keep the backend-version as it is and remove the frontend animation file, and further update the imports wherever the file has been used.
  2. Else, we can have a global util directory, where we can place those files or utility functions that will have the same functionality in the frontend and backend.

I think the second approach is better, because in this way the file directory will be more understandable for everyone. If we follow the first approach then there can be some ambiguity in the frontend side when we are doing imports from the backend and also suppose in the future if the backend and frontend are split into different repositories then that may break the code.

Please let me know which method you think will be best and then I can start working on it if @raj5036 have not started the work.

@Peallyz
Copy link

Peallyz commented Dec 20, 2022

Hi, is this issue still open ? May i work on it ?

@EmileGreyling
Copy link

Hi is this issue still open?

@shravan20
Copy link
Owner Author

Hi is this issue still open?

Yes, this is open

@codegeass56
Copy link

Hi. May I be assigned to work on this?

@shravan20
Copy link
Owner Author

Hi. May I be assigned to work on this?

Hey @codegeass56, feel free to pick this up and raise a PR. :)

@codegeass56
Copy link

Hi @shravan20,

I spent a few hours understanding the codebase. The issue boils down to the limitations of using create-react-app (CRA) which disallows importing a file from outside /src.

The most optimal solution (without affecting performance) is the existing one if you want to play by the rules of CRA.

Other solutions I have come across are:

  1. Modifying server logic to send the file and fetch it in the front-end. (Would not be as fast as keeping the animation logic hardcoded in the front-end)
  2. Removing ModuleScopePlugin entirely. (This would go against the rules of CRA and require significant testing)
  3. Using sym links. (Only works locally)

I am interested to see how this issue is fixed and will be following it. For now, I am unassigning myself and will try to contribute elsewhere.

I am a novice when it comes to full-stack apps so pray tell if you have any suggestions :)

@codegeass56 codegeass56 removed their assignment Jul 30, 2024
@EmileGreyling EmileGreyling removed their assignment Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation duplicate This issue or pull request already exists enhancement New feature or request good first issue Good for newcomers hacktoberfest hacktoberfest-accepted help wanted Extra attention is needed Little work This task takes little time to complete. (ETA couple of days) refactoring The changes in code without affecting functionality to improve the code
Projects
None yet
Development

No branches or pull requests

6 participants