Update all - query performance

[ebkr]

There was some code iterating dependencies in the previous version however it doesn't appear to actually be needed. The dependencies are still downloaded as expected and the visual indication of the mods that are required to be updated are identical.

Overall this saves around 5 seconds of hanging when uninstalling, disabling, and dragging items in the installed mod view.

The reason for the performance impact is due to the localModList state update also triggering a check to fetch available updates for the update all banner. We could probably further improve performance by not doing the check when the banner is no longer visible however the performance gain from that is neglible in comparison.

For TMM
As pointed out by @anttimaki this will need updating on the TMM implementation, however should be able to be copy/pasted without any changes for now.

[anttimaki]

Tested and reviewed, LGTM.

[anttimaki]

Here's some possible future iterations I came up with, any thougts?

Version comparison

Depending on the predicted future use cases, compareToDescending could be refined to:

1. A static method (a, b) => a.getVersionNumber().compareToDescending(b.getVersionNumber()), which could then be used like pkg.getVersions().sort(ThunderstoreMod.sortVersionsToDescending)
2. A method pkg.getLatestVersion()

Or either of those could be implemented separately, utilising compareToDescending. These are just for ease-of-use, and since there's currently no other use cases, this is not very important now.

Caching

To avoid having two "caches" (PACKAGES_MAP and LATEST_VERSIONS), we could do any of the following:

1. Add ThunderstoreMod.latestVersion field which is populated in handlePackageApiResponse in the same manner as LATEST_VERSIONS is done now
2. Like above, but use the existing versionNumber field, which ThunderstoreMod inherits from Mod via ThunderstoreVersion. This would be of type VersionNumber rather than ThunderstoreVersion, so there would be extra computation when getLatestOfAllToUpdate requires the latter.
3. Theoretically we could use ThunderstoreCombo in PACKAGES_MAP but this would require more refactoring elsewhere

[anttimaki]

Oh I was supposed to mention this too, but forgot when I got distracted by the thoughts in the earlier comment.

• This could probably use the getCachedThunderstoreModFromMod I added recently?
• I feel some of the changes here might be redundant to isCachedLatestVersion. IDK which place would make most sense for these, but having two somewhat competing implementations seems bad design? Something to think about in conjunction with caching-stuff mentioned above.

[ebkr]

This could probably use the getCachedThunderstoreModFromMod I added recently?

Yeah I agree, I'll change that

I feel some of the changes here might be redundant to isCachedLatestVersion. IDK which place would make most sense for these, but having two somewhat competing implementations seems bad design? Something to think about in conjunction with caching-stuff mentioned above.

What happens if the mod doesn't exist as part of the TS API (eg: locally imported)? I guess it'd return false, and then we try to ModBridge it and it wouldn't resolve to anything

[anttimaki]

What happens if the mod doesn't exist as part of the TS API (eg: locally imported)? I guess it'd return false, and then we try to ModBridge it and it wouldn't resolve to anything

I'm not sure I get what you're asking about, i.e. what happens at what part of code? The cached ModBridge implementation returns undefined instead of ThunderstoreMod when asking for an unknown mod, and true when asking if the unknown mod is at the latest version. Returning true for the latter would make sense to me, since we probably don't want to show "update this mod" when we have no idea if there's a newer version for it.

[ebkr]

@anttimaki Should be good for a re-review

[anttimaki]

The changes in the latest commit look good to me.