HeroicGamesLauncher icon indicating copy to clipboard operation
HeroicGamesLauncher copied to clipboard

Switch from react-scripts to Vite & clean up everything to work with strict mode

Open CommandMC opened this issue 3 years ago • 4 comments

  • Move from react-scripts/CRA to Vite
  • Clean up all (well, most of) the backend code to work with TS' strict mode
    • This involved a lot of refactoring and a (almost) completely rewritten LegendaryLibrary
    • Also contains most of the changes made in #1602

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/dist-mac/dist-win as always

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

  • [X] 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)

CommandMC avatar Jul 26 '22 02:07 CommandMC

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

CommandMC avatar Jul 31 '22 14:07 CommandMC

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

arielj avatar Jul 31 '22 14:07 arielj

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

CommandMC avatar Jul 31 '22 14:07 CommandMC

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.

arielj avatar Jul 31 '22 16:07 arielj

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

flavioislima avatar Aug 17 '22 18:08 flavioislima

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 avatar Aug 21 '22 16:08 CommandMC

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)

CommandMC avatar Aug 21 '22 16:08 CommandMC

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

flavioislima avatar Aug 25 '22 17:08 flavioislima

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 8754058cba8a0b6dbb1379bf28debd655b22cb11.
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

CommandMC avatar Aug 25 '22 18:08 CommandMC