Rename the lockfile types

[KSXGitHub]

From https://github.com/pnpm/pacquet/pull/143#discussion_r1348937221

@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.

[zkochan]

Other possible option: DependencyVariation (or DependencyVariant). I think I like this one most.

[KSXGitHub]

@zkochan

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"

Custom registry is at it again! I have yet to encountered a lockfile with custom registry so it's hard for me to formulate a correct schema and structure.

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.

I will need to think more about this, later.

[zkochan]

Custom registry is at it again! I have yet to encountered a lockfile with custom registry so it's hard for me to formulate a correct schema and structure.

Maybe we don't need custom registry support at all. I couldn't reproduce a lockfile that has them and I can't find related tests. I think they are not needed.

[KSXGitHub]

@zkochan Is there an issue or a PR that introduces custom registry as a package version specifier in the lockfile?

[zkochan]

Forget about custom registry in dependency path. It is legacy. We'll remove it from pnpm v9.

[KSXGitHub]

Forget about custom registry in dependency path. It is legacy. We'll remove it from pnpm v9.

Does it mean that dependency path (that is, the keys in the snapshot objects) would no longer start with / prefix?

[zkochan]

No, I don't think that will change.