Quasar upgrade #1823

ebkr · 2025-06-20T17:58:58Z

Upgrades to the latest version of Quasar, which gives us the following benefits:

Vue 3, which comes with:
- Security fixes
- Access to new Vue features such as Suspense and Teleport
- Better performance / lower memory usage
- Back onto a supported project as Vue 2 was EOL (Dec 31st, 2023).
Vite builds
- Performance is signficantly quicker (30-60s down to approx < 5s)
IPC calls
- This is a change that has been needed for an incredibly long time, but we've now been forced to implement IPC to call some of the internal node libraries. This is also a security change.
  - This is somewhat voided until we can figure out the CORS issue, however I don't think it's a huge concern for now.

There are an insane number of files changed. This PR is best reviewed on a per-commit basis, with comments addressing the relevant files on the main PR rather than individual commits, as there may be changes later that have already been reflected.

A large number of these files won't actually need much reviewing as they are things such as moved files (EG: Every game image has been moved), or imports being changed to use our new IPC based calls.

Rebase onto develop to keep game file location up to date

Lord-Kamina · 2025-07-23T22:01:54Z

I think you deleted some files that shouldn't have been deleted in 06adc95, like the babel and jest config files.

Also, you should probably update babel because you're using a version from roughly 6 years ago.

ebkr · 2025-07-24T07:09:54Z

I think you deleted some files that shouldn't have been deleted in 06adc95, like the babel and jest config files.

This was intentional as we no longer need Jest or Babel. It now uses Vite and so we're using Rollup, and for tests we're able to switch to Vitest or just use node:test.

Also, you should probably update babel because you're using a version from roughly 6 years ago.

If anything I just need to remove the dependency for it now. I'll do this as part of a review round.

Lord-Kamina · 2025-07-24T10:23:05Z

I think you deleted some files that shouldn't have been deleted in 06adc95, like the babel and jest config files.

This was intentional as we no longer need Jest or Babel. It now uses Vite and so we're using Rollup, and for tests we're able to switch to Vitest or just use node:test.

Also, you should probably update babel because you're using a version from roughly 6 years ago.

If anything I just need to remove the dependency for it now. I'll do this as part of a review round.

Oh ok. Nevermind then

JamesKoenig · 2025-07-26T17:01:44Z

This was intentional as we no longer need Jest or Babel. It now uses Vite and so we're using Rollup, and for tests we're able to switch to Vitest or just use node:test.
[...]
If anything I just need to remove the dependency for it now. I'll do this as part of a review round.

Is the clearing of the configurations supposed to be sort of like a redtag? To force the unit tests to fail until those dependency changes are addressed?

Like you said, the dependencies are still in `package.json`:

r2modmanPlus/package.json

Lines 74 to 75 in 04a868a

    
           "@babel/core": "^7.4.0", 
        
           "@babel/preset-typescript": "^7.14.5",

r2modmanPlus/package.json

Lines 91 to 92 in 04a868a

    
           "babel-core": "^7.0.0-beta.3", 
        
           "babel-jest": "^27.0.2",

r2modmanPlus/package.json

Lines 100 to 102 in 04a868a

    
           "eslint-plugin-jest": "^28.8.3", 
        
           "identity-obj-proxy": "^3.0.0", 
        
           "jest": "^29.7.0",

r2modmanPlus/package.json

Line 116 in 04a868a

"vue-jest": "^3.0.0",

And the `test:unit:ci` script still runs `jest --ci`:

r2modmanPlus/package.json

Line 22 in 04a868a

"test:unit:ci": "jest --ci",

Which given that is called in the github workflow:

r2modmanPlus/.github/workflows/test.yml

Lines 37 to 39 in 04a868a

    
                         run: > 
        
                             node test/folder-structure-testing/populator.mjs && 
        
                             yarn run test:unit:ci

This results in the two starting unit tests failing, while the latter two build tests succeed.

Along the same vein, would the unit tests like the ones in https://github.com/ebkr/r2modmanPlus/tree/develop/test/jest/__tests__ need to be migrated to vitest?

ebkr · 2025-07-28T09:29:18Z

Is the clearing of the configurations supposed to be sort of like a redtag? To force the unit tests to fail until those dependency changes are addressed?

Honestly no, it's just that I'd like this reviewed before I make any further changes. Tests aren't essential to the Quasar upgrade changes and can be done separately.

They're left-over for now because I haven't gotten to the test changes yet.

anttimaki

A collection of artisanal comments with varying importance
I omitted nitpicks like missing semicolons and stuff like that, and ignored pretty much all CSS changes
There were some TODO comments and console loggings that seemed they were intended to be cleaned up before a PR
My main interest at this point is:
1. Getting the project to run locally
2. Will TSMM work? Do you have a plan for
3. After the changes there seems to be a lot of warnings/errors on my IDE (Cursor) that bury the actual problems and weaken the type safety. Ideally those would be resolved one way or another and sooner rather than later

src-electron/ipc/node-fs-impl.ts

src-electron/ipc/zip-hook.ts

src-electron/preload/node-fs.ts

src/pages/Profiles.vue

src/providers/generic/zip/AdmZipProvider.ts

src/store/modules/SplashModule.ts

anttimaki · 2025-08-14T07:35:49Z

src/store/modules/TsModsModule.ts

@@ -192,6 +176,28 @@ export const TsModsModule = {
        },
        updateDeprecated(state, allMods: ThunderstoreMod[]) {
            state.deprecated = Deprecations.getDeprecatedPackageMap(allMods);
+        },
+        prewarmCacheMod(state: State, mods: ThunderstoreMod[]) {


I made an earlier comment about this method "no longer" being about prewarming the cache, but I confused it with the old prewarmCache below. Still, this is more about updating the cache so another name might be better?

anttimaki · 2025-08-14T07:37:43Z

src/store/modules/TsModsModule.ts

@@ -192,6 +176,28 @@ export const TsModsModule = {
        },
        updateDeprecated(state, allMods: ThunderstoreMod[]) {
            state.deprecated = Deprecations.getDeprecatedPackageMap(allMods);
+        },
+        prewarmCacheMod(state: State, mods: ThunderstoreMod[]) {
+            const localState = new Map<string, CachedMod>(state.cache.entries());


Just wondering if changing from mutating the cache Map to duplicating it might lead to performance issues on large profiles or race conditions if the commit is called in rapid succession 🤷🏻

This change was to mitigate a performance issue because it directly affects the cachedMod getter - Whenever the cache updates, the getter does a refresh too.

It's a valid concern about race conditions. I think we have the following options:

Wrap in a lock

IMO less ideal, but makes it easy to manage

Diff the cache and only re-set the changes - I'm unsure if calling .set re-triggers an update if the value is the same.

Keep the behaviour the same as before (always just call .set)

Something else?

Well to think of it, the mutation is synchronous and the environment is single-threaded by nature, and only other place that changes state.cache is clearModCache mutation, so maybe we're safe in race condition regard?

src/store/modules/TsModsModule.ts

Lord-Kamina · 2025-08-14T12:56:13Z

I don't know enough TS or JS to really make many meaningful comments here but just a minor BTW: In my local fork (of course not including this PR) I updated typescript to the latest 4.9.x and everything worked fine; in case doing so here might make any refactorings easier.

anttimaki

Generally speaking it seems there's still some places were code is wrapped in new promises explicitly, and where then-chains are used when not necessary, It's probably apparent from earlier comments but I'm not personally a fan of that approach. Not a blocker for this PR but we should probably at least have a discussion about the subject on how the matter should be handled in code and code reviews going forward.
When manually testing the dev build, page transitions feel a bit sluggish compared to pre-upgrade version. Like they take some 100-200ms longer. I might be hallucinating it. Or it could be vite doing some stuff JIT. Or the changes to the router? Have you noticed this?

src-electron/electron-main.ts

src/r2mm/launching/instructions/instructions/loader/CustomInstructions.ts

ebkr · 2025-08-21T15:16:34Z

When manually testing the dev build, page transitions feel a bit sluggish compared to pre-upgrade version. Like they take some 100-200ms longer. I might be hallucinating it. Or it could be vite doing some stuff JIT. Or the changes to the router? Have you noticed this?

No you're absolutely correct. I spent a while playing around with it and it appears to be a VueX issue which is what prompted me to bring up the discussion internally.

To recap:
A commit in the Vue 3 supported version of VueX takes approximately 150ms. If we move to something like globally exported reactives then we bring that number down to quite literally 5ms.

MobX has a greater overhead in terms of developer implementation time however the time to update is double that of globally exported reactives, but still more than acceptable at 10ms.

Lord-Kamina · 2025-08-26T01:20:47Z

So... I took a stab at removing all those promises within promises within promises and here's what I came up with.
Mind you, I can't figure out a way to test it other than launching the app, because in early stages, even something that I would have thought wouldn't compile just ran fine (but refused to work as expected)

For this patch, there could of course be errors (I'd be surprised if there aren't really) but I tried to launch, create a new profile, download some mods and export the profile as a file and seemingly, all that worked.

At the very least, it might give you a base to get rid of all those extra new Promise

0001-Remove-superfluous-promises.patch

ebkr · 2025-08-26T07:13:59Z

So... I took a stab at removing all those promises within promises within promises and here's what I came up with. Mind you, I can't figure out a way to test it other than launching the app, because in early stages, even something that I would have thought wouldn't compile just ran fine (but refused to work as expected)

For this patch, there could of course be errors (I'd be surprised if there aren't really) but I tried to launch, create a new profile, download some mods and export the profile as a file and seemingly, all that worked.

At the very least, it might give you a base to get rid of all those extra new Promise

0001-Remove-superfluous-promises.patch

Instead of doing a patch, do the following:

Fork the repository
Clone your fork
Apply the patch to your fork
Commit and push
Create a PR from your forked repository into this repository

That way:

You'll get to receive contributions identified by GitHub
I can review it easier
Any changes that are requested can be kept track of as part of the patch

…fied in the CDN

…lized

- Refactored stub providers from Jest to Vitest

- DownloadModule.ts duplicate action - Splash.vue formatting - Profiles.vue unused imports

ebkr force-pushed the quasar-upgrade branch from cfa0d49 to 6e35ea2 Compare June 26, 2025 11:09

ebkr requested a review from anttimaki July 23, 2025 13:22

ebkr marked this pull request as ready for review July 23, 2025 13:40

ebkr mentioned this pull request Jul 24, 2025

[FEATURE REQUEST] - Use a supported version of Electron #1661

Open

ebkr force-pushed the quasar-upgrade branch from 4cb61fb to bc8b70e Compare August 4, 2025 10:06

anttimaki reviewed Aug 14, 2025

View reviewed changes

ebkr force-pushed the quasar-upgrade branch from 802757e to eb7b068 Compare August 19, 2025 14:44

anttimaki reviewed Aug 21, 2025

View reviewed changes

src-electron/electron-main.ts Show resolved Hide resolved

src/r2mm/launching/instructions/instructions/loader/CustomInstructions.ts Show resolved Hide resolved

ebkr force-pushed the quasar-upgrade branch from 3469d43 to be776a8 Compare August 21, 2025 10:27

ebkr force-pushed the quasar-upgrade branch 2 times, most recently from a6a2cd3 to 0e36b51 Compare August 22, 2025 09:05

ebkr force-pushed the quasar-upgrade branch 2 times, most recently from 5eee176 to 56a8e0b Compare August 29, 2025 09:58

anttimaki mentioned this pull request Sep 1, 2025

Cover image optimization #1883

Open

anttimaki approved these changes Sep 9, 2025

View reviewed changes

ebkr added 6 commits September 12, 2025 10:56

"Quasar upgrade" dependency updates

e432fa6

Removed Quasar configuration files

0ce453e

Updated Quasar config file

00fce10

Added required index.html and updated packages

11243dc

Updated yarn.lock file

8e4f9b1

Resolved missing Quasar variable errors

91bac69

ebkr added 29 commits September 12, 2025 11:40

Changed UrlUtils to replace protocol to be in line with the one speci…

c1fdcdb

…fied in the CDN

Added transformer usages for hard-coded TS URLs

8f7d0b9

Fixed bug when importing profiles due to getEntries not being deseria…

eaebc1e

…lized

Fixed path import in WrapperArguments.ts

961c7d7

Changed DownloadUtils.ts to use IPC path

669546a

WIP: Vitest support

9775ce4

Fixed MelonLoader state.spec.ts

bdf691b

WIP: Added Installer tests, fixing ReturnOfModding.Tests.spec.ts

858427a

Ported GameInstructionGenerator.ts.spec.ts

d37457f

Ported Profile.ts.spec.ts

0bb258a

Ported Game installer tests

c07255f

Removed ported tests from Jest folder

fd3bf67

Ported InMemory.FsProvider.spec.ts to Vitest

1656284

Added provider for AppWindow calls to fix ModLinker test

d702d7d

- Refactored stub providers from Jest to Vitest

Ported Util tests to Vitest

0306b5d

Moved test-setup to Vitest

2faae24

Ported utils to Vitest

4ba650d

Removed Jest test folders

09ac7c0

Removed all Jest references

5ab240d

Removed process.stdout.write usages

42ed5d1

Updated package.json to run correct test command

cb69ab8

Bumped node version requirement in GitHub Actions build.yml

ed61823

Bumped node version requirement in GitHub Actions test.yml

fa61e19

Added Quasar Prepare step to generate tsconfig.json in GitHub Actions

63d85be

Updated yarn.lock after rebase

e7b0958

Fixed test after rebase

d952234

Addressed PR comments

f8d7ea5

Updated several path references to be relative

b3b0928

Fixed issues after rebase

dad6801

- DownloadModule.ts duplicate action - Splash.vue formatting - Profiles.vue unused imports

ebkr force-pushed the quasar-upgrade branch from 777cfe4 to dad6801 Compare September 12, 2025 14:44

Quasar upgrade #1823

Are you sure you want to change the base?

Quasar upgrade #1823

Uh oh!

Conversation

ebkr commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lord-Kamina commented Jul 23, 2025

Uh oh!

ebkr commented Jul 24, 2025

Uh oh!

Lord-Kamina commented Jul 24, 2025

Uh oh!

JamesKoenig commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Like you said, the dependencies are still in package.json:

And the test:unit:ci script still runs jest --ci:

Which given that is called in the github workflow:

Uh oh!

ebkr commented Jul 28, 2025

Uh oh!

anttimaki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anttimaki Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

anttimaki Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

ebkr Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

anttimaki Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lord-Kamina commented Aug 14, 2025

Uh oh!

anttimaki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ebkr commented Aug 21, 2025

Uh oh!

Lord-Kamina commented Aug 26, 2025

Uh oh!

ebkr commented Aug 26, 2025

Uh oh!

Uh oh!

ebkr commented Jun 20, 2025 •

edited

Loading

JamesKoenig commented Jul 26, 2025 •

edited

Loading

Like you said, the dependencies are still in `package.json`:

And the `test:unit:ci` script still runs `jest --ci`: