-
-
Notifications
You must be signed in to change notification settings - Fork 933
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
Changes from all commits
43a173c
befc765
bd7df76
d575cbc
5dee13d
ec7dfb2
4fee31d
07a2e87
ca6b6f8
93d5599
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# @pnpm/catalogs.protocol-parser | ||
|
||
## 0.1.0 | ||
|
||
Initial release |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# @pnpm/catalogs.protocol-parser | ||
|
||
> Parse catalog protocol specifiers and return the catalog name. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require('../../jest.config.js') |
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:*" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { parseCatalogProtocol } from './parseCatalogProtocol' |
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 | ||
} |
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') | ||
}) | ||
}) |
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": [] | ||
} |
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" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# @pnpm/catalogs.types | ||
|
||
## 0.1.0 | ||
|
||
Initial release |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# @pnpm/catalogs.types | ||
|
||
> Types related to the pnpm catalogs feature. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of creating a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good to me There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
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:*" | ||
} | ||
} |
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 | ||
} |
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": [] | ||
} |
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" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the catalogs:
default:
foo:
specifier: ^1.2.3
version: 1.2.3 This PR writes the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} |
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])) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,8 @@ export interface StrictInstallOptions { | |
|
||
supportedArchitectures?: SupportedArchitectures | ||
hoistWorkspacePackages?: boolean | ||
|
||
useBetaCatalogsFeat?: boolean | ||
Comment on lines
+147
to
+148
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Alternatively we could:
I thought the temporary |
||
} | ||
|
||
export type InstallOptions = | ||
|
There was a problem hiding this comment.
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.