Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement catalog protocol reads (feature flagged) #8020

Closed
wants to merge 10 commits into from
5 changes: 5 additions & 0 deletions catalogs/protocol-parser/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# @pnpm/catalogs.protocol-parser

## 0.1.0

Initial release
3 changes: 3 additions & 0 deletions catalogs/protocol-parser/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @pnpm/catalogs.protocol-parser
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, this small package gets used in:

  • pkg-manager/core/src/install/index.ts
  • pkg-manager/resolve-dependencies/src/getCatalogResolver.ts

Thought it was worth breaking it out into a new package. The publishing packages will likely need this utility too in a future PR.


> Parse catalog protocol specifiers and return the catalog name.
1 change: 1 addition & 0 deletions catalogs/protocol-parser/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('../../jest.config.js')
39 changes: 39 additions & 0 deletions catalogs/protocol-parser/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"name": "@pnpm/catalogs.protocol-parser",
"version": "0.1.0",
"description": "Parse catalog protocol specifiers and return the catalog name.",
"main": "lib/index.js",
"types": "lib/index.d.ts",
"engines": {
"node": ">=18.12"
},
"files": [
"lib",
"!*.map"
],
"repository": "https://github.com/pnpm/pnpm/blob/main/catalogs/protocol-parser",
"keywords": [
"pnpm9",
"pnpm",
"types"
],
"license": "MIT",
"bugs": {
"url": "https://github.com/pnpm/pnpm/issues"
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/catalogs/protocol-parser#readme",
"scripts": {
"lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"",
"compile": "tsc --build && pnpm run lint --fix",
"prepublishOnly": "pnpm run compile",
"test": "pnpm run compile && pnpm run _test",
"_test": "jest"
},
"funding": "https://opencollective.com/pnpm",
"exports": {
".": "./lib/index.js"
},
"devDependencies": {
"@pnpm/catalogs.protocol-parser": "workspace:*"
}
}
1 change: 1 addition & 0 deletions catalogs/protocol-parser/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { parseCatalogProtocol } from './parseCatalogProtocol'
18 changes: 18 additions & 0 deletions catalogs/protocol-parser/src/parseCatalogProtocol.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const CATALOG_PROTOCOL = 'catalog:'

/**
* Parse a package.json dependency specifier using the catalog: protocol.
* Returns null if the given specifier does not start with 'catalog:'.
*/
export function parseCatalogProtocol (pref: string): string | 'default' | null {
if (!pref.startsWith(CATALOG_PROTOCOL)) {
return null
}

const catalogNameRaw = pref.slice(CATALOG_PROTOCOL.length).trim()

// Allow a specifier of 'catalog:' to be a short-hand for 'catalog:default'.
const catalogNameNormalized = catalogNameRaw === '' ? 'default' : catalogNameRaw

return catalogNameNormalized
}
20 changes: 20 additions & 0 deletions catalogs/protocol-parser/test/parseCatalogProtocol.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { parseCatalogProtocol } from '@pnpm/catalogs.protocol-parser'

test('parses named catalog', () => {
expect(parseCatalogProtocol('catalog:foo')).toBe('foo')
expect(parseCatalogProtocol('catalog:bar')).toBe('bar')
})

test('returns null for specifier not using catalog protocol', () => {
expect(parseCatalogProtocol('^1.0.0')).toBe(null)
})

describe('default catalog', () => {
test('parses explicit default catalog', () => {
expect(parseCatalogProtocol('catalog:default')).toBe('default')
})

test('parses implicit catalog', () => {
expect(parseCatalogProtocol('catalog:')).toBe('default')
})
})
13 changes: 13 additions & 0 deletions catalogs/protocol-parser/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "@pnpm/tsconfig",
"composite": true,
"compilerOptions": {
"outDir": "lib",
"rootDir": "src"
},
"include": [
"src/**/*.ts",
"../../__typings__/**/*.d.ts"
],
"references": []
}
8 changes: 8 additions & 0 deletions catalogs/protocol-parser/tsconfig.lint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": "./tsconfig.json",
"include": [
"src/**/*.ts",
"test/**/*.ts",
"../../__typings__/**/*.d.ts"
]
}
5 changes: 5 additions & 0 deletions catalogs/types/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# @pnpm/catalogs.types

## 0.1.0

Initial release
3 changes: 3 additions & 0 deletions catalogs/types/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @pnpm/catalogs.types

> Types related to the pnpm catalogs feature.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new @pnpm/catalogs.types package, we could also stuff the small definitions here into the existing @pnpm/types package. I opted for a new package for now since we're doing something similar for hooks: @pnpm/hooks.types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll separate the new packages out to a different PR so you don't have to keep looking at these changes when re-reviewing this PR.

38 changes: 38 additions & 0 deletions catalogs/types/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"name": "@pnpm/catalogs.types",
"version": "0.1.0",
"description": "Types related to the pnpm catalogs feature.",
"main": "lib/index.js",
"types": "lib/index.d.ts",
"engines": {
"node": ">=18.12"
},
"files": [
"lib",
"!*.map"
],
"repository": "https://github.com/pnpm/pnpm/blob/main/catalogs/types",
"keywords": [
"pnpm9",
"pnpm",
"types"
],
"license": "MIT",
"bugs": {
"url": "https://github.com/pnpm/pnpm/issues"
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/catalogs/types#readme",
"scripts": {
"lint": "eslint \"src/**/*.ts\"",
"compile": "tsc --build && pnpm run lint --fix",
"prepublishOnly": "pnpm run compile",
"test": "pnpm run compile"
},
"funding": "https://opencollective.com/pnpm",
"exports": {
".": "./lib/index.js"
},
"devDependencies": {
"@pnpm/catalogs.types": "workspace:*"
}
}
29 changes: 29 additions & 0 deletions catalogs/types/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Catalogs parsed from the pnpm-workspace.yaml file.
*
* https://github.com/pnpm/rfcs/pull/1
*/
export interface Catalogs {
/**
* The default catalog.
*
* The default catalog can be defined in 2 ways.
*
* 1. Users can specify a top-level "catalog" field or,
* 2. An explicitly named "default" catalog under the "catalogs" map.
*
* This field contains either definition. Note that it's an error to define
* the default catalog using both options. The parser will fail when reading
* the workspace manifest.
*/
readonly default?: Catalog

/**
* Named catalogs.
*/
readonly [catalogName: string]: Catalog | undefined
}

export interface Catalog {
readonly [dependencyName: string]: string | undefined
}
13 changes: 13 additions & 0 deletions catalogs/types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "@pnpm/tsconfig",
"composite": true,
"compilerOptions": {
"outDir": "lib",
"rootDir": "src"
},
"include": [
"src/**/*.ts",
"../../__typings__/**/*.d.ts"
],
"references": []
}
8 changes: 8 additions & 0 deletions catalogs/types/tsconfig.lint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": "./tsconfig.json",
"include": [
"src/**/*.ts",
"test/**/*.ts",
"../../__typings__/**/*.d.ts"
]
}
3 changes: 3 additions & 0 deletions lockfile/lockfile-file/src/lockfileFormatConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ function normalizeLockfile (lockfile: InlineSpecifiersLockfile, opts: NormalizeL
if (lockfileToSave.time) {
lockfileToSave.time = pruneTimeInLockfileV6(lockfileToSave.time, lockfile.importers ?? {})
}
if ((lockfileToSave.catalogs != null) && isEmpty(lockfileToSave.catalogs)) {
delete lockfileToSave.catalogs
}
if ((lockfileToSave.overrides != null) && isEmpty(lockfileToSave.overrides)) {
delete lockfileToSave.overrides
}
Expand Down
10 changes: 10 additions & 0 deletions lockfile/lockfile-file/src/sortLockfileKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type RootKey = keyof LockfileFile
const ROOT_KEYS: readonly RootKey[] = [
'lockfileVersion',
'settings',
'catalogs',
'overrides',
'packageExtensionsChecksum',
'pnpmfileChecksum',
Expand Down Expand Up @@ -86,6 +87,15 @@ export function sortLockfileKeys (lockfile: LockfileFileV9): LockfileFileV9 {
})
}
}
if (lockfile.catalogs != null) {
lockfile.catalogs = sortKeys(lockfile.catalogs)
for (const [catalogName, catalog] of Object.entries(lockfile.catalogs)) {
lockfile.catalogs[catalogName] = sortKeys(catalog, {
compare: lexCompare,
deep: true,
})
}
}
for (const key of ['dependencies', 'devDependencies', 'optionalDependencies', 'time', 'patchedDependencies'] as const) {
if (!lockfile[key]) continue
lockfile[key] = sortKeys<any>(lockfile[key]) // eslint-disable-line @typescript-eslint/no-explicit-any
Expand Down
22 changes: 22 additions & 0 deletions lockfile/lockfile-types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface Lockfile {
importers: Record<string, ProjectSnapshot>
lockfileVersion: string
time?: Record<string, string>
catalogs?: CatalogSnapshots
packages?: PackageSnapshots
overrides?: Record<string, string>
packageExtensionsChecksum?: string
Expand Down Expand Up @@ -135,3 +136,24 @@ export type PackageBin = string | { [name: string]: string }
* }
*/
export type ResolvedDependencies = Record<string, string>

export interface CatalogSnapshots {
[catalogName: string]: { [dependencyName: string]: ResolvedCatalogEntry }
}

export interface ResolvedCatalogEntry {
/**
* The real specifier that should be used for this dependency's catalog entry.
* This would be the ^1.2.3 portion of:
*
* @example
* catalog:
* foo: ^1.2.3
*/
readonly specifier: string

/**
* The concrete version that the requested specifier resolved to. Ex: 1.2.3
*/
readonly version: string
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the pnpm-lock.yaml file, this will look like:

catalogs:
  default:
    foo:
      specifier: ^1.2.3
      version: 1.2.3

This PR writes the version part. A future PR will read from it to optimize new usages of the catalog protocol. I think the logic will be similar to the existing replaceVersionInPref logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if there will be multiple named catalogs with the same specs? will the entries be duplicated? Like

catalogs:
  default:
    foo:
      specifier: ^1.2.3
      version: 1.2.3
  named:
    foo:
      specifier: ^1.2.3
      version: 1.2.3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, they'll be duplicated. I think that's okay and somewhat by design if someone duplicates a catalog entry across multiple named catalogs.

}
2 changes: 2 additions & 0 deletions pkg-manager/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
"@pnpm/build-modules": "workspace:*",
"@pnpm/builder.policy": "3.0.0",
"@pnpm/calc-dep-state": "workspace:*",
"@pnpm/catalogs.protocol-parser": "workspace:*",
"@pnpm/catalogs.types": "workspace:*",
"@pnpm/constants": "workspace:*",
"@pnpm/core-loggers": "workspace:*",
"@pnpm/crypto.base32-hash": "workspace:*",
Expand Down
1 change: 1 addition & 0 deletions pkg-manager/core/src/getPeerDependencyIssues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export async function getPeerDependencyIssues (
} = await resolveDependencies(
projectsToResolve,
{
catalogs: ctx.catalogs,
currentLockfile: ctx.currentLockfile,
allowedDeprecatedVersions: {},
allowNonAppliedPatches: false,
Expand Down
11 changes: 11 additions & 0 deletions pkg-manager/core/src/install/allCatalogsAreUpToDate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { type CatalogSnapshots } from '@pnpm/lockfile-file'
import { type Catalogs } from '@pnpm/catalogs.types'

export function allCatalogsAreUpToDate (
catalogsConfig: Catalogs,
snapshot: CatalogSnapshots | undefined
): boolean {
return Object.entries(snapshot ?? {})
.every(([catalogName, catalog]) => Object.entries(catalog ?? {})
.every(([alias, entry]) => entry.specifier === catalogsConfig[catalogName]?.[alias]))
}
2 changes: 2 additions & 0 deletions pkg-manager/core/src/install/extendInstallOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ export interface StrictInstallOptions {

supportedArchitectures?: SupportedArchitectures
hoistWorkspacePackages?: boolean

useBetaCatalogsFeat?: boolean
Comment on lines +147 to +148
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for a setting? It is opt-in anyway, as the new field in pnpm-workspace.yaml should be added.

Copy link
Member Author

@gluxon gluxon Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mostly to avoid bug reports that catalogs aren't working on the next version (ex: 9.1.0). I was thinking it'd be better for it to not work entirely rather than partially work.

Without the setting here, the catalog: protocol would work when defined pnpm-workspace.yaml, but we wouldn't replace it on publish. I'm working on the publish replacing part next.

Alternatively we could:

  • Use an environment variable to feature flag this.
  • Avoid merging any catalog PRs until it's completely finished.

I thought the temporaryuseBetaCatalogsFeat argument was the best out of those 3 options, but open to other thoughts!

}

export type InstallOptions =
Expand Down