GMOD / jbrowse-components

Source code for JBrowse 2, a modern React-based genome browser
https://jbrowse.org/jb2
Apache License 2.0
205 stars 61 forks source link

Fix name indexing on desktop #4572

Closed cmdcolin closed 1 week ago

cmdcolin commented 1 week ago

The name indexing was producing errors on desktop in a way that was actually related to the typescript types not catching an error

there was a argument like this

const indexingParams = {
      outLocation,
      tracks,
      exclude,
      attributes,
      assemblies,
      indexType,
      statusCallback,
      signal,
    }
    await indexTracks(indexingParams)

however, many the parameter names had been renamed (outLocation->outDir, exclude->featureTypesToExclude, attributes->attributesToIndex) at some point and it was not catching that this was an error

however, changing the code to

    await indexTracks({
      outLocation,
      tracks,
      exclude,
      attributes,
      assemblies,
      indexType,
      statusCallback,
      signal,
    })

this instantly produces the typescript errors about providing keys that the function does not want, e.g. it does not want the outLocation, it does not want the exclude, etc.

this is a good hint to audit and purge of any type of 'pre-declare the argument separately from the function call' usages in the codebase

fixes #4570

cmdcolin commented 1 week ago

this PR also proposes putting all the trix indexes in the appdata folder, with foldername based on +Date.now() timestamp

previously, the trix index folder was colocated with the "session file" location.

i think the old behavior is hazardous and could lead to permission denied errors, annoyances with files being created, and even collisions with previous sessions. the downside is one might have more trouble 'finding' the trix files but there is not really a system for e.g. converting a desktop session to a web session (with folder-relative files) in previous versions, so i think there is no 'change in expectation' from this new behavior.

cmdcolin commented 1 week ago

also, for the record, the above typescript-not-catching-this error is quite surprising. i cannot reproduce it in the typescript playground

https://www.typescriptlang.org/play/?#code/FAMwrgdgxgLglgewgAgCYIMozCEAKAbwA8AaALwF8AuYqiMAWwCMBTAJ3LsdbYoEpkBYMhHIoSAM4IANiwB00hAHM8pMn2AVgoSLEQp0WHCABMhUgE9qtes3YkLXO7wFDRYyTPmKVljVvEICRhkIioAQwgLAF4AFmBA4ORHSJiAZgTJEIQmACto4gctbUNsXDwc3I1S4zNKjWAgA

cmdcolin commented 1 week ago

random follow up: the explanation of why it didn't catch these issues was because all those params that were being passed wrong were optional args