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

[Tech] Switch from react-scripts to Vite & clean up everything to work with strict mode #1633

Merged
merged 26 commits into from Aug 25, 2022

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented Jul 26, 2022

Tested everything on Linux so far, will test on Windows as well. Mac testing would be appreciated as always

The way development works now is the following:

  • If you want to quickly test new things, you run yarn dev. Heroic will open and any change you make in the frontend or backend will reload/restart it.
  • If you want to build, you run yarn dist:linux/dist:mac/dist:win as always
  • If you want to make sure that no code violates TS rules, you run yarn codecheck.
    Note: This is different from how it worked before. In the interest of speed & de-duplication, Vite does not check for violations itself since it's (1) way slower to do so and (2) you should already know about them through some other tool (VSCode shows them for example).
    If you want to be absolutely sure everything's fine, you can run this command to (try to) compile every .ts[x] file in the project with tsc

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

 - Rename "src" -> "src/frontend"
 - Rename "electron" -> "src/backend"
 - Move types.ts to "common/types.ts"
This just cleans up package.json a bit
@CommandMC CommandMC added the pr:wip WIP, don't merge. label Jul 26, 2022
@CommandMC CommandMC self-assigned this Jul 26, 2022
@CommandMC CommandMC changed the title Switch from react-scripts + foreman + webpack + ts-loader -> Vite Switch from react-scripts to Vite & clean up everything to work with strict mode Jul 31, 2022
@CommandMC CommandMC marked this pull request as ready for review July 31, 2022 14:09
@CommandMC CommandMC added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:wip WIP, don't merge. labels Jul 31, 2022
@CommandMC
Copy link
Collaborator Author

Since there are a lot of changes here, feel free to only review code you've written/you know yourself & leave the parts you don't know that well up to someone else. Last thing I want is this not getting reviewed because of its size

@arielj
Copy link
Collaborator

arielj commented Jul 31, 2022

Since there are a lot of changes here, feel free to only review code you've written/you know yourself & leave the parts you don't know that well up to someone else. Last thing I want is this not getting reviewed because of its size

good thing is that most file changes are just renames or changes in the imports to point to the right place, so it looks bigger than what it really is

I check this, I didn't look at code I've never touched before, I think it looks good at least the parts I understand

I'll try the branch later and use the app for a while to see if there's anything I can find broken

One question, I think we are removing the unreal market stuff, right? I see a lot of is_game checks begin removed

EDIT: I can't imagine the amount of conflicts you get here xD

@CommandMC
Copy link
Collaborator Author

One question, I think we are removing the unreal market stuff, right? I see a lot of is_game checks begin removed

Yes, I got rid of that since I don't think it was working anymore. Could add it back if it's really necessary though, although I really don't think anyone used it

EDIT: I can't imagine the amount of conflicts you get here xD

It's fine actually, Git itself is much better at auto-resolving things than GitHub

@arielj
Copy link
Collaborator

arielj commented Jul 31, 2022

One question, I think we are removing the unreal market stuff, right? I see a lot of is_game checks begin removed

Yes, I got rid of that since I don't think it was working anymore. Could add it back if it's really necessary though, although I really don't think anyone used it

I've never used that either (I think it was actually broken, it never showed anything for me but maybe I just don't know how to use that feature). Just wonder if anybody actually uses that, maybe there was a discussion about this already in discord.

Turns out refactoring a lot of stuff can introduce issues, who would've
thought
You never have to run this yourself, so it was a bit confusing to have.
If you really need its functionality, `vite build` isn't that hard to
type in on your own :^)
Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

Let's merge this after resolving conflicts. We need this Pr to progress further with refactoring. Bugs can be fixed later here. This pr is the first step of beta 2.5.0

@flavioislima
Copy link
Member

I will start to review and test this one. Might take a few days though.

This command can be used to make sure that no files in the project
violate TS rules, since Vite itself doesn't check them

It would probably be a good idea to add them to GH Workflows and/or
pre-commit hooks, but I'm not too sure on how to do that
Not sure how this happens, but it seems they've gotten screwed up
in the merge
@CommandMC
Copy link
Collaborator Author

One issue I've now encountered is that updates don't seem to want to work. They run through fine, but the frontend still displays an update even after it's done. Backend does not report this game as update-able, so it's definitely some caching weirdness

I'm not too sure if this is caused by this PR, but it's definitely what I'm gonna look into next

@CommandMC
Copy link
Collaborator Author

Another thing I'd like to move away from is watching Legendary's installed.json. If the file's updated, it's written in chunks, so when the first chunk gets written & we try to read it, it'll be malformed.
Instead, I'd like to really only read it out if we need to (so on startup & after installing/importing/updating/uninstalling games)

This fixes an issue where refreshing the library while only showing
one library made the update icons of all the others disappear
src/backend/main.ts Show resolved Hide resolved
@flavioislima flavioislima changed the title Switch from react-scripts to Vite & clean up everything to work with strict mode [Tech] Switch from react-scripts to Vite & clean up everything to work with strict mode Aug 25, 2022
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

What colossal work, thanks for that.
From what I saw and tested I could not find any bug, even the one you said about the update status not updating. For me it works fine.

Vite is also a lot better to develop and does everything faster so it was a pretty good improvement.

In my opinion it is ready to merge on beta so if some bug appears we can fix it afterwards :)

@flavioislima flavioislima added the pr:ready-to-merge This PR is fully ready for merge. label Aug 25, 2022
@flavioislima
Copy link
Member

@Nocccer I believe you need to dismiss the requested changes, otherwise, it can't be merged.

Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

Pretty good work. I alread developed a little bit against it with the logger refactor.

@flavioislima flavioislima merged commit 2ba6604 into beta Aug 25, 2022
@flavioislima flavioislima deleted the feat/vite branch August 25, 2022 17:23
@CommandMC
Copy link
Collaborator Author

From what I saw and tested I could not find any bug, even the one you said about the update status not updating. For me it works fine.

Well, that would be because I fixed it in 8754058.
I just discarded the old update list in the state of the frontend if checkUpdates was true, instead of appending what the backend sent to it. I assume the original change was done to make it possible to refresh the updates of just one library, although as you saw (well, you didn't, but I did), it introduces this "stuck updates" issue if the frontend (for whatever reason) doesn't remove the game from the updateable list

flavioislima added a commit that referenced this pull request Sep 13, 2022
* [Feat] Proper check if heroic can communicate with internet (#1677)

* Added ping to check internet connection

* removed console.log

* Make log of error more clear

* Remove ping package and just use ping cli command

* Return if one ping succeed

* [i18n] Updated Translations (#1728)

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translation: Heroic Games Launcher/GamePage

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translation: Heroic Games Launcher/GamePage

* Translated using Weblate (Czech)

Currently translated at 100.0% (358 of 358 strings)

Translated using Weblate (Czech)

Currently translated at 100.0% (358 of 358 strings)

Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Co-authored-by: Shimon <simonfarm0@gmail.com>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/globals/cs/
Translation: Heroic Games Launcher/GamePage
Translation: Heroic Games Launcher/Globals

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translation: Heroic Games Launcher/GamePage

* Translated using Weblate (Korean)

Currently translated at 100.0% (358 of 358 strings)

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Co-authored-by: Moon Sungjoon <sumoon@seoulsaram.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/globals/ko/
Translation: Heroic Games Launcher/Globals

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translation: Heroic Games Launcher/GamePage

* Translated using Weblate (Russian)

Currently translated at 100.0% (358 of 358 strings)

Translated using Weblate (Russian)

Currently translated at 100.0% (358 of 358 strings)

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Co-authored-by: Sedative <tertre3@gmail.com>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/globals/ru/
Translation: Heroic Games Launcher/Globals

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translation: Heroic Games Launcher/GamePage

* Translated using Weblate (Persian)

Currently translated at 100.0% (358 of 358 strings)

Update translation files

Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Hosted Weblate <hosted@weblate.org>
Co-authored-by: Parsa Shadab <shadab.balgori@gmail.com>
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/gamepage/
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/globals/fa/
Translation: Heroic Games Launcher/GamePage
Translation: Heroic Games Launcher/Globals

* Update translation files

Updated by "Squash Git commits" hook in Weblate.

Translation: Heroic Games Launcher/Globals
Translate-URL: https://hosted.weblate.org/projects/heroic-games-launcher/globals/

Co-authored-by: Shimon <simonfarm0@gmail.com>
Co-authored-by: Moon Sungjoon <sumoon@seoulsaram.org>
Co-authored-by: Sedative <tertre3@gmail.com>
Co-authored-by: Parsa Shadab <shadab.balgori@gmail.com>

* Revert "[i18n] Updated Translations" (#1745)

Revert "[i18n] Updated Translations (#1728)"

This reverts commit 7f5d2b3.

* [Tech] Switch from react-scripts to Vite & clean up everything to work with strict mode (#1633)

* Change complete project structure

 - Rename "src" -> "src/frontend"
 - Rename "electron" -> "src/backend"
 - Move types.ts to "common/types.ts"

* Switch from react-scripts + foreman + webpack + ts-loader -> Vite

* Set GH_TOKEN and GITHUB_TOKEN in tests directly

This just cleans up package.json a bit

* Get SVG & JPG importing working in VSCode

* Move type definitions into common/typedefs

* Fixup tests

* Rewrite large chunks of the backend to be compatible with strict mode

* Modify frontend to reflect changes to type definition structure

* Remove unused files

These weren't necessary, at least as far as I can tell

* Update translations

* Fixup: Re-add `--filesystem` parameter for Proton

Turns out refactoring a lot of stuff can introduce issues, who would've
thought

* Remove `build` script

You never have to run this yourself, so it was a bit confusing to have.
If you really need its functionality, `vite build` isn't that hard to
type in on your own :^)

* Fully remove all remnants of UE support

* Add new 'codecheck' command

This command can be used to make sure that no files in the project
violate TS rules, since Vite itself doesn't check them

It would probably be a good idea to add them to GH Workflows and/or
pre-commit hooks, but I'm not too sure on how to do that

* Update translation files

* Fix translation files

Not sure how this happens, but it seems they've gotten screwed up
in the merge

* Remove some more unnecessary files

* Always get game updates from all libraries

This fixes an issue where refreshing the library while only showing
one library made the update icons of all the others disappear

* Some misc code style changes

* Merge branch 'main' of github.com:Heroic-Games-Launcher/HeroicGamesLauncher into beta

* [Fix] Check if Legendary's `metadata` folder exists before trying to read it (#1785)

* [Fix] Not permanently saving downloadNoHttps config option (#1797)

fixing not permanently saving downloadNoHttps config option

* Merge branch 'main' of github.com:Heroic-Games-Launcher/HeroicGamesLauncher into beta

* [Fix] TypeError: Unexpected end of json (#1806)

* remove useEffect double import

* catch type error end of json

* Added try catch

* Add log prefix to see what runner fails

* [Tech] Add codecheck to pre push and workflow (#1794)

* Add codecheck to pre push and workflow

* Add new workflow for codecheck

* renaming

* [Fix] Update legendary binaries (#1809)

Update legendary binaries

* fix: gog getGameinfo after clear the cache

Co-authored-by: Niklas <61798668+Nocccer@users.noreply.github.com>
Co-authored-by: Weblate (bot) <hosted@weblate.org>
Co-authored-by: Shimon <simonfarm0@gmail.com>
Co-authored-by: Moon Sungjoon <sumoon@seoulsaram.org>
Co-authored-by: Sedative <tertre3@gmail.com>
Co-authored-by: Parsa Shadab <shadab.balgori@gmail.com>
Co-authored-by: Mathis Dröge <34034631+CommandMC@users.noreply.github.com>
Co-authored-by: Jan B <32649612+Keksgesicht@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P pr:ready-to-merge This PR is fully ready for merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants