Skip to content

Rename the lockfile types #146

@KSXGitHub

Description

@KSXGitHub

From #143 (comment)

@anonrig

PkgNameVerPeer is really ambiguous. I PackageNameVersionPeer also doesn't make sense as a struct name.

@KSXGitHub

PkgNameVerPeer vs PackageNameVersionPeer preference aside. Naming this was particularly difficult for me, because I had to follow the lockfile format as defined in pnpm.

In pnpm, there are peculiar anonymous string types with defined syntax. The pnpm repo doesn't give them a name, only use string and parse it later (which is error-prone).

In Rust, I made it parses at the same time as serde. Though the names of these data structure is not clear as their purposes are overlapped and reused. Perhaps @zkochan can help us find better names for these Pkg* types?

@zkochan

I am not sure. Dependency path could be a good name because this is basically the directory name inside the virtual store. But dep path is already used for this + the custom registry. Maybe ShortDependencyPath? Then the one currently called dependency path would become FullDependencyPath?

And by the way, a short dependency path can be only a registry-hosted package. githosted and other types of dependencies have a different format for the dependency path.

@KSXGitHub

Dependency path could be a good name because this is basically the directory name inside the virtual store.

It's not exactly the names in the virtual store. The names in the lockfile uses ( and ), but they were all replaced with _. Furthermore, the names in the lockfile contain / for scoped names. I originally name it DependencyPath because the spec calls the keys of the snapshot object "dependency path".

TBH, I don't think this is a good name. It doesn't refer to any actual path, and it fails to describe the purpose of the type as well as the components.

Concatenating the components of a syntax together may have been a temporary solution at the time, but now that I think about it, it's the best solution so far.

And by the way, a short dependency path can be only a registry-hosted package. githosted and other types of dependencies have a different format for the dependency path.

Does this mean that this "short dependency path" should be an enum and name+ver+peer is a variant in it?

@zkochan

It's not exactly the names in the virtual store. The names in the lockfile uses ( and ), but they were all replaced with _. Furthermore, the names in the lockfile contain / for scoped names.

Like you said, it is the directory name with some conversion to make it a valid directory name.

Concatenating the components of a syntax together may have been a temporary solution at the time, but now that I think about it, it's the best solution so far.

You mean naming it PkgNameVerPeers? But then you also need to add PatchHash to it because it also can contain a patch hash.

Does this mean that this "short dependency path" should be an enum and name+ver+peer is a variant in it?

no, because you separated the custom registry field from name+ver+peer and custom registry is only related to name+ver+peer, not other types of "dependency path"

Maybe it can be named "Dependency ID"? We currently use Package ID to name name+ver. So name+ver+peers would be Dependency ID. But it could also be PkgSnapshotID as the entries in the lockfile are called PackageSnapshot.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions