Remove any-types and Mod class #1204
Conversation
| @@ -142,8 +141,7 @@ import SearchAndSort from './LocalModList/SearchAndSort.vue'; | |||
| } | |||
| } | |||
|
|
|||
| getMissingDependencies(vueMod: any): string[] { | |||
| const mod: Mod = new Mod().fromReactive(vueMod); | |||
There was a problem hiding this comment.
Do we have any issues from having dropped the fromReactive call?
There was a problem hiding this comment.
My next performance improvement branch is based on this branch, so I've been monkey testing it quite lot, and so far I've not seen any problems in (dev) building the project, errors logged in the console, or weird behaviour in the UI itself. But work on that continues so I still have more changes to observe these changes as well.
However, I'm not entirely sure in what situations should we expect the objects to get serialized and cause problems in the first place?
There was a problem hiding this comment.
It's related to loading the profile from the mods.yml file. ManifestV2 is serialized whenever ProfileModList is called to save, and is deserialized whenever we load the contents again
There was a problem hiding this comment.
This is probably good to double check, and if it does prove to still be a potential issue, in my opinion the proper deserialization should simply be moved to be a part of the loading process.
There was a problem hiding this comment.
Seems like this should not be an issue, the source of the data seems to all trace back to ProfileModList.getModList which also instantiates a proper object for it rather than just passing in the values. This change just simply adds the proper type annotation here and in doing so makes it apparent that the Mod class is not actually needed anymore.
It is possible that the vuex state somehow ends up containing something other than it's annotated to do without our current TS configuration catching it (or e.g. due to type casting that should not be done somewhere), but I couldn't find any such code at least (as mentioned, most seems to originate from ProfileModList.getModList, or alternatively some of the mutation operations in the same module)
There was a problem hiding this comment.
Alright so the main difference here is that the Mod.fromReactive implementation uses the actual package manifest.json for populating the values, whereas the ProfileModList.getModList uses whatever happens to be populated in the mods.yml. In a sense the Mod.fromReactive implementation behaves more "correctly" in this case as it's actually using the data provided by the package itself rather than a state cache file.
In practice I'm not sure if it matters, but I do think the correct way to load mod metadata is to read it from said mod's own manifest file. I suppose the changes in this PR are still an improvement because it's better to have the metadata parsing be consistent and predictable, which is being achieved by removing the rarely used alternative metadata parsing.
anttimaki
force-pushed
the
mod-list-sort
branch
from
93a2f02
to
21bce97
Compare
In practice all mod arguments are of ManifestV2 type. Refer to this type consistently as "mod", and use "tsMod" for the few occurances of ThunderstoreMod types.
The class was no longer used directly, only by ThunderstoreVersion which extends it. Relevant attributes and getters/setters were moved to ThunderstoreVersion definition. Mod.fromManifest was dropped since it's not used and did return a Mod object anyway.
anttimaki
force-pushed
the
mod-typing
branch
from
5ba2cad
to
bbd7ea6
Compare
No description provided.