-
-
Notifications
You must be signed in to change notification settings - Fork 439
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
[Ref/Feat] Clean up our routing and make it faster #3540
Conversation
This brings Heroic in line with modern react-router usage and allows for some new features
The code for this is a little verbose, but it enables Vite to split code into multiple files. Build output before (note the large file sizes): build/assets/index.8e7c2c97.js 2.58 KiB / gzip: 1.01 KiB build/assets/index.ae6fe98a.js 564.94 KiB / gzip: 181.67 KiB build/assets/App.63d9df39.js 1208.01 KiB / gzip: 361.96 KiB Build output after: build/assets/index.4b57bd40.js 10.33 KiB / gzip: 3.71 KiB build/assets/index.dda1a522.js 10.12 KiB / gzip: 3.92 KiB build/assets/index.22363332.js 7.89 KiB / gzip: 3.02 KiB build/assets/index.d02459a2.js 4.05 KiB / gzip: 1.62 KiB build/assets/index.c7dbdf3e.js 3.56 KiB / gzip: 1.25 KiB build/assets/index.51987511.js 2.63 KiB / gzip: 1.02 KiB build/assets/index.11c6e1ab.js 41.67 KiB / gzip: 13.69 KiB build/assets/index.f9487201.js 350.03 KiB / gzip: 109.55 KiB build/assets/App.f1d932b1.js 417.92 KiB / gzip: 125.16 KiB build/assets/index.886bf77e.js 385.92 KiB / gzip: 111.35 KiB build/assets/index.55a76c73.js 564.94 KiB / gzip: 181.67 KiB This means that on Heroic startup, Electron has to run "only" ~570KiB of JavaScript instead of ~1.2MiB. In my tests, this speeds up startup time by roughly 10%
this is probably a leftover from when I implemented the feature to store the last url for the stores that then needed some tweaks I think it's not used anymore. It's easy to check though, if the feature to ropen the stores in the last url works as expected it's all good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
Our Routes were a bit of an organically grown mess, with different approaches to do the same things, and generally using outdated React Router features. This PR cleans those up, and improves Heroic startup time in the process.
I did remove one route when "converting" from Route elements to objects:
/last-url
, I couldn't find any components navigating to it, so it seems like this was always unreachable?Other than that, I think we should look into making our WebView component less reliant on specific
location.pathname
values, instead just telling it what it's supposed to do (I've opted to choose params over props there, but I don't have any preference really, just seemed more convenient). Didn't want to touch too many things before a release though.Use the following Checklist if you have changed something on the Backend or Frontend: