From b22fd55900e117c699d6f6278a6d77231c3ccb01 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 31 Dec 2024 20:41:26 +0200 Subject: [PATCH 01/46] WIP: prototype extraction in its own manager --- lib/modules/manager/api.ts | 2 + .../manager/pnpm-catalog/extract.spec.ts | 74 +++++++++ lib/modules/manager/pnpm-catalog/extract.ts | 152 ++++++++++++++++++ lib/modules/manager/pnpm-catalog/index.ts | 24 +++ lib/modules/manager/pnpm-catalog/readme.md | 4 + 5 files changed, 256 insertions(+) create mode 100644 lib/modules/manager/pnpm-catalog/extract.spec.ts create mode 100644 lib/modules/manager/pnpm-catalog/extract.ts create mode 100644 lib/modules/manager/pnpm-catalog/index.ts create mode 100644 lib/modules/manager/pnpm-catalog/readme.md diff --git a/lib/modules/manager/api.ts b/lib/modules/manager/api.ts index 573fcf95375e93..d9e5e2ab08c4ef 100644 --- a/lib/modules/manager/api.ts +++ b/lib/modules/manager/api.ts @@ -78,6 +78,7 @@ import * as pipCompile from './pip-compile'; import * as pip_requirements from './pip_requirements'; import * as pip_setup from './pip_setup'; import * as pipenv from './pipenv'; +import * as pnpmCatalog from './pnpm-catalog'; import * as poetry from './poetry'; import * as preCommit from './pre-commit'; import * as pub from './pub'; @@ -175,6 +176,7 @@ api.set('mix', mix); api.set('nix', nix); api.set('nodenv', nodenv); api.set('npm', npm); +api.set('pnpmCatalog', pnpmCatalog); api.set('nuget', nuget); api.set('nvm', nvm); api.set('ocb', ocb); diff --git a/lib/modules/manager/pnpm-catalog/extract.spec.ts b/lib/modules/manager/pnpm-catalog/extract.spec.ts new file mode 100644 index 00000000000000..02cfb504438f6b --- /dev/null +++ b/lib/modules/manager/pnpm-catalog/extract.spec.ts @@ -0,0 +1,74 @@ +import { fs } from '../../../../test/util'; +import { extractAllPackageFiles } from './extract'; + +jest.mock('../../../util/fs'); + +describe('modules/manager/pnpm-catalog/extract', () => { + describe('extractAllPackageFiles()', () => { + it('ignores non pnpm-workspace.yaml files', async () => { + expect(await extractAllPackageFiles({}, ['package.json'])).toEqual([]); + }); + + it('ignores invalid pnpm-workspace.yaml file', async () => { + fs.readLocalFile.mockResolvedValueOnce('invalid'); + expect(await extractAllPackageFiles({}, ['pnpm-workspace.yaml'])).toEqual( + [], + ); + }); + + it('handles empty catalog', async () => { + fs.readLocalFile.mockResolvedValueOnce(` +catalog: +catalogs: +`); + expect(await extractAllPackageFiles({}, ['pnpm-workspace.yaml'])).toEqual( + [], + ); + }); + + it('parses valid pnpm-workspace.yaml file', async () => { + fs.readLocalFile.mockResolvedValueOnce( + ` +catalog: + react: 18.3.0 + +catalogs: + react17: + react: 17.0.2 +`, + ); + expect( + await extractAllPackageFiles({}, ['pnpm-workspace.yaml']), + ).toMatchObject([ + { + deps: [ + { + currentValue: '18.3.0', + datasource: 'npm', + depName: 'react', + depType: 'catalogDependency', + prettyDepType: 'catalogDependency', + managerData: { + catalogType: 'default', + catalogName: 'default', + }, + }, + { + currentValue: '17.0.2', + datasource: 'npm', + depName: 'react', + depType: 'catalogDependency', + prettyDepType: 'catalogDependency', + managerData: { + catalogType: 'name', + catalogName: 'react17', + }, + }, + ], + lockFiles: ['pnpm-workspace.yaml'], + packageFile: 'pnpm-workspace.yaml', + }, + ]); + }); + }); +}); diff --git a/lib/modules/manager/pnpm-catalog/extract.ts b/lib/modules/manager/pnpm-catalog/extract.ts new file mode 100644 index 00000000000000..1229296a5b5863 --- /dev/null +++ b/lib/modules/manager/pnpm-catalog/extract.ts @@ -0,0 +1,152 @@ +import { z } from 'zod'; +import { logger } from '../../../logger'; +import { readLocalFile } from '../../../util/fs'; +import { parseSingleYaml } from '../../../util/yaml'; + +import { + extractDependency, + parseDepName, +} from '../npm/extract/common/dependency'; +import { setNodeCommitTopic } from '../npm/extract/common/node'; +import type { NpmPackageDependency } from '../npm/extract/types'; +import type { NpmManagerData } from '../npm/types'; +import type { + ExtractConfig, + PackageDependency, + PackageFile, + PackageFileContent, +} from '../types'; + +function matchesFileName(fileNameWithPath: string, fileName: string): boolean { + return ( + fileNameWithPath === fileName || fileNameWithPath.endsWith(`/${fileName}`) + ); +} + +// TODO(fpapado): consider writing extractPackageFile first, then looping (with +// post- hooks) in extractAllPackageFiles + +export async function extractAllPackageFiles( + config: ExtractConfig, + matchedFiles: string[], +): Promise { + const packageFiles: PackageFile[] = []; + for (const matchedFile of matchedFiles) { + if (!matchesFileName(matchedFile, 'pnpm-workspace.yaml')) { + logger.warn({ matchedFile }, 'Invalid pnpm-workspace.yaml match'); + continue; + } + // TODO: Consider what to do about workspace: specifiers + let pnpmCatalogs: PnpmCatalogs; + try { + const content = (await readLocalFile(matchedFile, 'utf8'))!; + pnpmCatalogs = parsePnpmCatalogs(content); + } catch (err) { + logger.debug({ err, matchedFile }, 'Error parsing pnpm-workspace.yaml'); + continue; + } + + const packageFile = matchedFile; + const extracted = extractPnpmCatalogDeps(pnpmCatalogs); + + if (!extracted) { + logger.debug({ packageFile }, 'No dependencies found'); + continue; + } + + const res: PackageFile = { + ...extracted, + packageFile, + lockFiles: [matchedFile], + }; + + packageFiles.push(res); + } + + return packageFiles; +} + +/** + * A pnpm catalog is either the default catalog (catalog:, catlog:default), or a + * named one (under catalogs:) + */ +type CatalogType = + | { type: 'default'; name: 'default' } + | { type: 'named'; name: string }; + +type PnpmCatalogs = Array; + +const CATALOG_DEPENDENCY = 'catalogDependency'; + +export interface PnpmCatalogManagerData extends Record { + // TODO(fpapado): thread the pnpm-lock.yaml location; we will likely need it + // for updating + pnpmShrinkwrap?: string; + // TODO(fpapado): we likely want other fields from the npm manager data here, + // such as `skipInstalls` +} + +function extractPnpmCatalogDeps( + catalogs: PnpmCatalogs, +): PackageFileContent | null { + const deps: PackageDependency[] = []; + + for (const catalog of catalogs) { + for (const [key, val] of Object.entries(catalog.dependencies)) { + const depName = parseDepName('catalog', key); + let dep: PackageDependency = { + depType: CATALOG_DEPENDENCY, + depName, + managerData: { + // we assign the name of the catalog, in order to know what fields to + // update later on + catalogType: catalog.type, + catalogName: catalog.name, + }, + }; + if (depName !== key) { + dep.managerData!.key = key; + } + + // TODO: fix type #22198 + // FIXME(fpapado): small crime with `val!` + dep = { ...dep, ...extractDependency(CATALOG_DEPENDENCY, depName, val!) }; + // TODO(fpapado): verify whether we need this + setNodeCommitTopic(dep); + dep.prettyDepType = CATALOG_DEPENDENCY; + deps.push(dep); + } + } + + logger.info(deps); + + return { + deps, + }; +} + +const pnpmCatalogsSchema = z.object({ + catalog: z.optional(z.record(z.string())), + catalogs: z.optional(z.record(z.record(z.string()))), +}); + +function parsePnpmCatalogs(content: string): PnpmCatalogs { + const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = + parseSingleYaml(content, { customSchema: pnpmCatalogsSchema }); + + return [ + { + type: 'default', + name: 'default', + dependencies: defaultCatalogDeps ?? {}, + }, + ...Object.entries(namedCatalogs ?? {}).map( + ([name, dependencies]) => + ({ + type: 'named', + name, + dependencies, + }) as const, + ), + ]; +} diff --git a/lib/modules/manager/pnpm-catalog/index.ts b/lib/modules/manager/pnpm-catalog/index.ts new file mode 100644 index 00000000000000..d091f26949b522 --- /dev/null +++ b/lib/modules/manager/pnpm-catalog/index.ts @@ -0,0 +1,24 @@ +import type { Category } from '../../../constants'; +import { GithubTagsDatasource } from '../../datasource/github-tags'; +import { NpmDatasource } from '../../datasource/npm'; + +export { extractAllPackageFiles } from './extract'; +export { getRangeStrategy, updateDependency } from '../npm'; + +export const categories: Category[] = ['js']; + +export const defaultConfig = { + fileMatch: ['(^|/)pnpm-workspace\\.yaml$'], + digest: { + prBodyDefinitions: { + Change: + '{{#if displayFrom}}`{{{displayFrom}}}` -> {{else}}{{#if currentValue}}`{{{currentValue}}}` -> {{/if}}{{/if}}{{#if displayTo}}`{{{displayTo}}}`{{else}}`{{{newValue}}}`{{/if}}', + }, + }, + prBodyDefinitions: { + Change: + "[{{#if displayFrom}}`{{{displayFrom}}}` -> {{else}}{{#if currentValue}}`{{{currentValue}}}` -> {{/if}}{{/if}}{{#if displayTo}}`{{{displayTo}}}`{{else}}`{{{newValue}}}`{{/if}}]({{#if depName}}https://renovatebot.com/diffs/npm/{{replace '/' '%2f' depName}}/{{{currentVersion}}}/{{{newVersion}}}{{/if}})", + }, +}; + +export const supportedDatasources = [GithubTagsDatasource.id, NpmDatasource.id]; diff --git a/lib/modules/manager/pnpm-catalog/readme.md b/lib/modules/manager/pnpm-catalog/readme.md new file mode 100644 index 00000000000000..807c7f8435ef33 --- /dev/null +++ b/lib/modules/manager/pnpm-catalog/readme.md @@ -0,0 +1,4 @@ +Used for updating bun projects. +Bun is a tool for JavaScript projects and therefore an alternative to managers like npm, pnpm and Yarn. + +If a `package.json` is found to be part of `bun` manager results then the same file will be excluded from the `npm` manager results unless an npm/pnpm/Yarn lock file is also found. From 610add083cb65871639e1fd2577ab8796d164116 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 11:04:00 +0200 Subject: [PATCH 02/46] Move pnpm catalog extraction to npm manager --- lib/modules/manager/npm/extract/pnpm.spec.ts | 60 +++++++ lib/modules/manager/npm/extract/pnpm.ts | 119 +++++++++++++- lib/modules/manager/npm/extract/types.ts | 2 + .../manager/pnpm-catalog/extract.spec.ts | 74 --------- lib/modules/manager/pnpm-catalog/extract.ts | 152 ------------------ lib/modules/manager/pnpm-catalog/index.ts | 24 --- lib/modules/manager/pnpm-catalog/readme.md | 4 - 7 files changed, 179 insertions(+), 256 deletions(-) delete mode 100644 lib/modules/manager/pnpm-catalog/extract.spec.ts delete mode 100644 lib/modules/manager/pnpm-catalog/extract.ts delete mode 100644 lib/modules/manager/pnpm-catalog/index.ts delete mode 100644 lib/modules/manager/pnpm-catalog/readme.md diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 73ad5be0f51330..2f82d2a76ed209 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -1,3 +1,4 @@ +import { codeBlock } from 'common-tags'; import { Fixtures } from '../../../../../test/fixtures'; import { fs, getFixturePath, logger, partial } from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; @@ -7,6 +8,7 @@ import type { NpmManagerData } from '../types'; import { detectPnpmWorkspaces, extractPnpmFilters, + extractPnpmWorkspaceFile, findPnpmWorkspace, getPnpmLock, } from './pnpm'; @@ -284,4 +286,62 @@ describe('modules/manager/npm/extract/pnpm', () => { expect(res.lockedVersionsWithPath).toBeUndefined(); }); }); + + describe('.extractPnpmWorkspaceFile()', () => { + it('ignores invalid pnpm-workspace.yaml file', () => { + expect(extractPnpmWorkspaceFile('', 'pnpm-workspace.yaml')).toBeNull(); + }); + + it('handles empty catalog entries', () => { + expect( + extractPnpmWorkspaceFile( + codeBlock` + catalog: + catalogs: + `, + 'pnpm-workspace.yaml', + ), + ).toBeNull(); + }); + + it('parses valid pnpm-workspace.yaml file', () => { + expect( + extractPnpmWorkspaceFile( + codeBlock` + catalog: + react: 18.3.0 + + catalogs: + react17: + react: 17.0.2 + `, + 'pnpm-workspace.yaml', + ), + ).toMatchObject({ + deps: [ + { + currentValue: '18.3.0', + datasource: 'npm', + depName: 'react', + depType: 'catalogDependency', + prettyDepType: 'catalogDependency', + managerData: { + catalogName: 'default', + }, + }, + { + currentValue: '17.0.2', + datasource: 'npm', + depName: 'react', + depType: 'catalogDependency', + prettyDepType: 'catalogDependency', + managerData: { + catalogName: 'react17', + }, + }, + ], + packageFile: 'pnpm-workspace.yaml', + }); + }); + }); }); diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 4871ed0fc5baa4..a226f57a81e2d2 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -1,6 +1,7 @@ import is from '@sindresorhus/is'; import { findPackages } from 'find-packages'; import upath from 'upath'; +import { z } from 'zod'; import { GlobalConfig } from '../../../../config/global'; import { logger } from '../../../../logger'; import { @@ -10,10 +11,20 @@ import { readLocalFile, } from '../../../../util/fs'; import { parseSingleYaml } from '../../../../util/yaml'; -import type { PackageFile } from '../../types'; +import type { + ExtractConfig, + PackageDependency, + PackageFile, + PackageFileContent, +} from '../../types'; import type { PnpmDependencySchema, PnpmLockFile } from '../post-update/types'; import type { NpmManagerData } from '../types'; -import type { LockFile, PnpmWorkspaceFile } from './types'; +import { extractDependency, parseDepName } from './common/dependency'; +import type { + LockFile, + NpmPackageDependency, + PnpmWorkspaceFile, +} from './types'; function isPnpmLockfile(obj: any): obj is PnpmLockFile { return is.plainObject(obj) && 'lockfileVersion' in obj; @@ -222,3 +233,107 @@ function getLockedDependencyVersions( return res; } + +/** + * A pnpm catalog is either the default catalog (catalog:, catlog:default), or a + * named one (under catalogs:) + */ +type PnpmCatalog = { name: string; dependencies: NpmPackageDependency }; + +export function extractPnpmWorkspaceFile( + content: string, + packageFile: string, +): PackageFile | null { + logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); + + let pnpmCatalogs: Array; + try { + pnpmCatalogs = parsePnpmCatalogs(content); + } catch { + logger.debug({ packageFile }, `Invalid pnpm workspace YAML.`); + return null; + } + + const extracted = extractPnpmCatalogDeps(pnpmCatalogs); + + if (!extracted) { + logger.debug({ packageFile }, 'No dependencies found'); + return null; + } + + logger.debug(extracted, 'Extracted catalog dependencies.'); + + return { + ...extracted, + packageFile, + }; +} + +function extractPnpmCatalogDeps( + catalogs: Array, +): PackageFileContent | null { + const CATALOG_DEPENDENCY = 'catalogDependency'; + + const deps: PackageDependency[] = []; + + for (const catalog of catalogs) { + for (const [key, val] of Object.entries(catalog.dependencies)) { + const depName = parseDepName('catalog', key); + let dep: PackageDependency = { + depType: CATALOG_DEPENDENCY, + // TODO(fpapado): consider how users might be able to match on specific + // catalogs, such as catalog.default.react or catalog.react17.react + depName, + managerData: { + // we assign the name of the catalog, in order to know what fields to + // update later on + catalogName: catalog.name, + }, + }; + if (depName !== key) { + dep.managerData!.key = key; + } + + // TODO: fix type #22198 + dep = { + ...dep, + ...extractDependency( + CATALOG_DEPENDENCY, + depName, + // FIXME(fpapado): small crime with `val!` + val!, + ), + prettyDepType: CATALOG_DEPENDENCY, + }; + dep.prettyDepType = CATALOG_DEPENDENCY; + deps.push(dep); + } + } + + return { + deps, + }; +} + +const pnpmCatalogsSchema = z.object({ + catalog: z.optional(z.record(z.string())), + catalogs: z.optional(z.record(z.record(z.string()))), +}); + +function parsePnpmCatalogs(content: string): Array { + const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = + parseSingleYaml(content, { customSchema: pnpmCatalogsSchema }); + + return [ + { + name: 'default', + dependencies: defaultCatalogDeps ?? {}, + }, + ...Object.entries(namedCatalogs ?? {}).map(([name, dependencies]) => ({ + name, + dependencies, + })), + ]; +} + +// TODO: When extracting, use {depType: 'catalogDependency', depName: 'name.packageName'} diff --git a/lib/modules/manager/npm/extract/types.ts b/lib/modules/manager/npm/extract/types.ts index a9681aec758eb1..8b5c3ef7754236 100644 --- a/lib/modules/manager/npm/extract/types.ts +++ b/lib/modules/manager/npm/extract/types.ts @@ -36,6 +36,8 @@ export interface LockFile { export interface PnpmWorkspaceFile { packages: string[]; + catalog?: Partial>; + catalogs?: Partial>>>; } export type OverrideDependency = Record; diff --git a/lib/modules/manager/pnpm-catalog/extract.spec.ts b/lib/modules/manager/pnpm-catalog/extract.spec.ts deleted file mode 100644 index 02cfb504438f6b..00000000000000 --- a/lib/modules/manager/pnpm-catalog/extract.spec.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { fs } from '../../../../test/util'; -import { extractAllPackageFiles } from './extract'; - -jest.mock('../../../util/fs'); - -describe('modules/manager/pnpm-catalog/extract', () => { - describe('extractAllPackageFiles()', () => { - it('ignores non pnpm-workspace.yaml files', async () => { - expect(await extractAllPackageFiles({}, ['package.json'])).toEqual([]); - }); - - it('ignores invalid pnpm-workspace.yaml file', async () => { - fs.readLocalFile.mockResolvedValueOnce('invalid'); - expect(await extractAllPackageFiles({}, ['pnpm-workspace.yaml'])).toEqual( - [], - ); - }); - - it('handles empty catalog', async () => { - fs.readLocalFile.mockResolvedValueOnce(` -catalog: -catalogs: -`); - expect(await extractAllPackageFiles({}, ['pnpm-workspace.yaml'])).toEqual( - [], - ); - }); - - it('parses valid pnpm-workspace.yaml file', async () => { - fs.readLocalFile.mockResolvedValueOnce( - ` -catalog: - react: 18.3.0 - -catalogs: - react17: - react: 17.0.2 -`, - ); - expect( - await extractAllPackageFiles({}, ['pnpm-workspace.yaml']), - ).toMatchObject([ - { - deps: [ - { - currentValue: '18.3.0', - datasource: 'npm', - depName: 'react', - depType: 'catalogDependency', - prettyDepType: 'catalogDependency', - managerData: { - catalogType: 'default', - catalogName: 'default', - }, - }, - { - currentValue: '17.0.2', - datasource: 'npm', - depName: 'react', - depType: 'catalogDependency', - prettyDepType: 'catalogDependency', - managerData: { - catalogType: 'name', - catalogName: 'react17', - }, - }, - ], - lockFiles: ['pnpm-workspace.yaml'], - packageFile: 'pnpm-workspace.yaml', - }, - ]); - }); - }); -}); diff --git a/lib/modules/manager/pnpm-catalog/extract.ts b/lib/modules/manager/pnpm-catalog/extract.ts deleted file mode 100644 index 1229296a5b5863..00000000000000 --- a/lib/modules/manager/pnpm-catalog/extract.ts +++ /dev/null @@ -1,152 +0,0 @@ -import { z } from 'zod'; -import { logger } from '../../../logger'; -import { readLocalFile } from '../../../util/fs'; -import { parseSingleYaml } from '../../../util/yaml'; - -import { - extractDependency, - parseDepName, -} from '../npm/extract/common/dependency'; -import { setNodeCommitTopic } from '../npm/extract/common/node'; -import type { NpmPackageDependency } from '../npm/extract/types'; -import type { NpmManagerData } from '../npm/types'; -import type { - ExtractConfig, - PackageDependency, - PackageFile, - PackageFileContent, -} from '../types'; - -function matchesFileName(fileNameWithPath: string, fileName: string): boolean { - return ( - fileNameWithPath === fileName || fileNameWithPath.endsWith(`/${fileName}`) - ); -} - -// TODO(fpapado): consider writing extractPackageFile first, then looping (with -// post- hooks) in extractAllPackageFiles - -export async function extractAllPackageFiles( - config: ExtractConfig, - matchedFiles: string[], -): Promise { - const packageFiles: PackageFile[] = []; - for (const matchedFile of matchedFiles) { - if (!matchesFileName(matchedFile, 'pnpm-workspace.yaml')) { - logger.warn({ matchedFile }, 'Invalid pnpm-workspace.yaml match'); - continue; - } - // TODO: Consider what to do about workspace: specifiers - let pnpmCatalogs: PnpmCatalogs; - try { - const content = (await readLocalFile(matchedFile, 'utf8'))!; - pnpmCatalogs = parsePnpmCatalogs(content); - } catch (err) { - logger.debug({ err, matchedFile }, 'Error parsing pnpm-workspace.yaml'); - continue; - } - - const packageFile = matchedFile; - const extracted = extractPnpmCatalogDeps(pnpmCatalogs); - - if (!extracted) { - logger.debug({ packageFile }, 'No dependencies found'); - continue; - } - - const res: PackageFile = { - ...extracted, - packageFile, - lockFiles: [matchedFile], - }; - - packageFiles.push(res); - } - - return packageFiles; -} - -/** - * A pnpm catalog is either the default catalog (catalog:, catlog:default), or a - * named one (under catalogs:) - */ -type CatalogType = - | { type: 'default'; name: 'default' } - | { type: 'named'; name: string }; - -type PnpmCatalogs = Array; - -const CATALOG_DEPENDENCY = 'catalogDependency'; - -export interface PnpmCatalogManagerData extends Record { - // TODO(fpapado): thread the pnpm-lock.yaml location; we will likely need it - // for updating - pnpmShrinkwrap?: string; - // TODO(fpapado): we likely want other fields from the npm manager data here, - // such as `skipInstalls` -} - -function extractPnpmCatalogDeps( - catalogs: PnpmCatalogs, -): PackageFileContent | null { - const deps: PackageDependency[] = []; - - for (const catalog of catalogs) { - for (const [key, val] of Object.entries(catalog.dependencies)) { - const depName = parseDepName('catalog', key); - let dep: PackageDependency = { - depType: CATALOG_DEPENDENCY, - depName, - managerData: { - // we assign the name of the catalog, in order to know what fields to - // update later on - catalogType: catalog.type, - catalogName: catalog.name, - }, - }; - if (depName !== key) { - dep.managerData!.key = key; - } - - // TODO: fix type #22198 - // FIXME(fpapado): small crime with `val!` - dep = { ...dep, ...extractDependency(CATALOG_DEPENDENCY, depName, val!) }; - // TODO(fpapado): verify whether we need this - setNodeCommitTopic(dep); - dep.prettyDepType = CATALOG_DEPENDENCY; - deps.push(dep); - } - } - - logger.info(deps); - - return { - deps, - }; -} - -const pnpmCatalogsSchema = z.object({ - catalog: z.optional(z.record(z.string())), - catalogs: z.optional(z.record(z.record(z.string()))), -}); - -function parsePnpmCatalogs(content: string): PnpmCatalogs { - const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = - parseSingleYaml(content, { customSchema: pnpmCatalogsSchema }); - - return [ - { - type: 'default', - name: 'default', - dependencies: defaultCatalogDeps ?? {}, - }, - ...Object.entries(namedCatalogs ?? {}).map( - ([name, dependencies]) => - ({ - type: 'named', - name, - dependencies, - }) as const, - ), - ]; -} diff --git a/lib/modules/manager/pnpm-catalog/index.ts b/lib/modules/manager/pnpm-catalog/index.ts deleted file mode 100644 index d091f26949b522..00000000000000 --- a/lib/modules/manager/pnpm-catalog/index.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { Category } from '../../../constants'; -import { GithubTagsDatasource } from '../../datasource/github-tags'; -import { NpmDatasource } from '../../datasource/npm'; - -export { extractAllPackageFiles } from './extract'; -export { getRangeStrategy, updateDependency } from '../npm'; - -export const categories: Category[] = ['js']; - -export const defaultConfig = { - fileMatch: ['(^|/)pnpm-workspace\\.yaml$'], - digest: { - prBodyDefinitions: { - Change: - '{{#if displayFrom}}`{{{displayFrom}}}` -> {{else}}{{#if currentValue}}`{{{currentValue}}}` -> {{/if}}{{/if}}{{#if displayTo}}`{{{displayTo}}}`{{else}}`{{{newValue}}}`{{/if}}', - }, - }, - prBodyDefinitions: { - Change: - "[{{#if displayFrom}}`{{{displayFrom}}}` -> {{else}}{{#if currentValue}}`{{{currentValue}}}` -> {{/if}}{{/if}}{{#if displayTo}}`{{{displayTo}}}`{{else}}`{{{newValue}}}`{{/if}}]({{#if depName}}https://renovatebot.com/diffs/npm/{{replace '/' '%2f' depName}}/{{{currentVersion}}}/{{{newVersion}}}{{/if}})", - }, -}; - -export const supportedDatasources = [GithubTagsDatasource.id, NpmDatasource.id]; diff --git a/lib/modules/manager/pnpm-catalog/readme.md b/lib/modules/manager/pnpm-catalog/readme.md deleted file mode 100644 index 807c7f8435ef33..00000000000000 --- a/lib/modules/manager/pnpm-catalog/readme.md +++ /dev/null @@ -1,4 +0,0 @@ -Used for updating bun projects. -Bun is a tool for JavaScript projects and therefore an alternative to managers like npm, pnpm and Yarn. - -If a `package.json` is found to be part of `bun` manager results then the same file will be excluded from the `npm` manager results unless an npm/pnpm/Yarn lock file is also found. From c09e7bcc2d243a25fdcc1085492a842c198ef9b0 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 15:50:44 +0200 Subject: [PATCH 03/46] Remove pnpmCatalog from api.ts --- lib/modules/manager/api.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/modules/manager/api.ts b/lib/modules/manager/api.ts index d9e5e2ab08c4ef..c4219509abe229 100644 --- a/lib/modules/manager/api.ts +++ b/lib/modules/manager/api.ts @@ -176,7 +176,6 @@ api.set('mix', mix); api.set('nix', nix); api.set('nodenv', nodenv); api.set('npm', npm); -api.set('pnpmCatalog', pnpmCatalog); api.set('nuget', nuget); api.set('nvm', nvm); api.set('ocb', ocb); From a3a52508e323f09b3a7c5f2c018f00d885bf2aca Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 22:01:22 +0200 Subject: [PATCH 04/46] Implement updatePnpmCatalogDependency in updateDependency --- lib/modules/manager/npm/extract/index.ts | 24 +++-- lib/modules/manager/npm/extract/pnpm.ts | 12 +-- lib/modules/manager/npm/index.ts | 3 +- .../manager/npm/update/dependency/index.ts | 101 ++++++++++++++++++ lib/util/yaml.ts | 1 + 5 files changed, 128 insertions(+), 13 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index 0b9fbba90638e0..18cc86c024f003 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -17,6 +17,7 @@ import type { import type { NpmLockFiles, NpmManagerData } from '../types'; import { getExtractedConstraints } from './common/dependency'; import { extractPackageJson } from './common/package-file'; +import { extractPnpmWorkspaceFile } from './pnpm'; import { postExtract } from './post'; import type { NpmPackage } from './types'; import { isZeroInstall } from './yarn'; @@ -229,12 +230,23 @@ export async function extractAllPackageFiles( const content = await readLocalFile(packageFile, 'utf8'); // istanbul ignore else if (content) { - const deps = await extractPackageFile(content, packageFile, config); - if (deps) { - npmFiles.push({ - ...deps, - packageFile, - }); + // TODO(fpapado): consider whether to do this here, or in the postExtract step + if (packageFile === 'pnpm-workspace.yaml') { + const deps = extractPnpmWorkspaceFile(content, packageFile); + if (deps) { + npmFiles.push({ + ...deps, + packageFile, + }); + } + } else { + const deps = await extractPackageFile(content, packageFile, config); + if (deps) { + npmFiles.push({ + ...deps, + packageFile, + }); + } } } else { logger.debug({ packageFile }, `No content found`); diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index a226f57a81e2d2..f7fa7b51e3c95d 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -12,7 +12,6 @@ import { } from '../../../../util/fs'; import { parseSingleYaml } from '../../../../util/yaml'; import type { - ExtractConfig, PackageDependency, PackageFile, PackageFileContent, @@ -97,8 +96,8 @@ export async function detectPnpmWorkspaces( const packagePathCache = new Map(); for (const p of packageFiles) { - const { packageFile, managerData } = p; - const { pnpmShrinkwrap } = managerData as NpmManagerData; + const { packageFile, managerData = {} } = p; + const { pnpmShrinkwrap } = managerData as Partial; // check if pnpmShrinkwrap-file has already been provided if (pnpmShrinkwrap) { @@ -243,6 +242,7 @@ type PnpmCatalog = { name: string; dependencies: NpmPackageDependency }; export function extractPnpmWorkspaceFile( content: string, packageFile: string, + // TODO: consider config: ExtractConfig here ): PackageFile | null { logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); @@ -272,13 +272,13 @@ export function extractPnpmWorkspaceFile( function extractPnpmCatalogDeps( catalogs: Array, ): PackageFileContent | null { - const CATALOG_DEPENDENCY = 'catalogDependency'; + const CATALOG_DEPENDENCY = 'pnpm.catalog'; const deps: PackageDependency[] = []; for (const catalog of catalogs) { for (const [key, val] of Object.entries(catalog.dependencies)) { - const depName = parseDepName('catalog', key); + const depName = parseDepName(CATALOG_DEPENDENCY, key); let dep: PackageDependency = { depType: CATALOG_DEPENDENCY, // TODO(fpapado): consider how users might be able to match on specific @@ -315,7 +315,7 @@ function extractPnpmCatalogDeps( }; } -const pnpmCatalogsSchema = z.object({ +export const pnpmCatalogsSchema = z.object({ catalog: z.optional(z.record(z.string())), catalogs: z.optional(z.record(z.record(z.string()))), }); diff --git a/lib/modules/manager/npm/index.ts b/lib/modules/manager/npm/index.ts index 9385b7606c20f6..0834cb36d966ad 100644 --- a/lib/modules/manager/npm/index.ts +++ b/lib/modules/manager/npm/index.ts @@ -20,7 +20,8 @@ export const url = 'https://docs.npmjs.com'; export const categories: Category[] = ['js']; export const defaultConfig = { - fileMatch: ['(^|/)package\\.json$'], + // TODO: Consider this vs. a post hook + fileMatch: ['(^|/)package\\.json$', '(^|/)pnpm-workspace\\.yaml$'], digest: { prBodyDefinitions: { Change: diff --git a/lib/modules/manager/npm/update/dependency/index.ts b/lib/modules/manager/npm/update/dependency/index.ts index b6e1ca0735a0b3..9bb7deb2b396e6 100644 --- a/lib/modules/manager/npm/update/dependency/index.ts +++ b/lib/modules/manager/npm/update/dependency/index.ts @@ -1,9 +1,12 @@ import is from '@sindresorhus/is'; import { dequal } from 'dequal'; +import { parseDocument } from 'yaml'; import { logger } from '../../../../../logger'; import { escapeRegExp, regEx } from '../../../../../util/regex'; import { matchAt, replaceAt } from '../../../../../util/string'; +import { parseSingleYaml } from '../../../../../util/yaml'; import type { UpdateDependencyConfig, Upgrade } from '../../../types'; +import { pnpmCatalogsSchema } from '../../extract/pnpm'; import type { DependenciesMeta, NpmPackage, @@ -28,6 +31,12 @@ function renameObjKey( }, {} as DependenciesMeta); } +// TODO(fpapado): implement this +// eslint-disable-next-line @typescript-eslint/no-unused-vars +function replaceYaml(): string { + return 'TODO'; +} + function replaceAsString( parsedContents: NpmPackage, fileContent: string, @@ -111,10 +120,102 @@ function replaceAsString( throw new Error(); } +// TODO(fpapado): move this somewhere else, e.g. npm/update/dependency/pnpm +function updatePnpmCatalogDependency({ + fileContent, + upgrade, +}: UpdateDependencyConfig): string | null { + const { depType, managerData, depName } = upgrade; + + const catalogName = managerData?.catalogName; + + if (!is.string(catalogName)) { + logger.error( + 'No catalogName was found; this is likely an extractoin error.', + ); + return null; + } + + let { newValue } = upgrade; + + // Handle git tags or digests + if (upgrade.currentRawValue) { + if (upgrade.currentDigest) { + logger.debug('Updating package.json git digest'); + newValue = upgrade.currentRawValue.replace( + upgrade.currentDigest, + // TODO #22198 + + upgrade.newDigest!.substring(0, upgrade.currentDigest.length), + ); + } else { + logger.debug('Updating package.json git version tag'); + newValue = upgrade.currentRawValue.replace( + upgrade.currentValue, + upgrade.newValue, + ); + } + } + + // Handle aliases + if (upgrade.npmPackageAlias) { + newValue = `npm:${upgrade.packageName}@${newValue}`; + } + + logger.info( + `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, + ); + + // Save the old version + let parsedContents; + + try { + // TODO: we should move pnpmCatalogsSchema around to a common/shared location in the npm manager + parsedContents = parseSingleYaml(fileContent, { + customSchema: pnpmCatalogsSchema, + }); + } catch (err) { + logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); + return null; + } + + const oldVersion = + catalogName === 'default' + ? parsedContents.catalog?.[depName!] + : parsedContents.catalogs?.[catalogName]?.[depName!]; + + if (oldVersion === newValue) { + logger.trace('Version is already updated'); + return fileContent; + } + + // TODO: Consider separating `parseSingleYaml` and + // `parseSingleYamlDocument`/`yamlDocumentToJS`, so that we can keep the unified API + const document = parseDocument(fileContent, { + uniqueKeys: false, + strict: false, + }); + + if (catalogName === 'default') { + // TODO: We should check whether `catalogs\n:default:\npkg: 1.0.0` is valid, + // because in that case we need to know whether it was set in `catalog` or + // `catalogs\ndefault` + document.setIn(['catalog', depName], newValue); + } else { + document.setIn(['catalogs', catalogName, depName], newValue); + } + + return document.toString(); +} + export function updateDependency({ fileContent, upgrade, }: UpdateDependencyConfig): string | null { + if (upgrade.depType === 'pnpm.catalog') { + return updatePnpmCatalogDependency({ fileContent, upgrade }); + } + const { depType, managerData } = upgrade; const depName: string = managerData?.key || upgrade.depName; let { newValue } = upgrade; diff --git a/lib/util/yaml.ts b/lib/util/yaml.ts index e8d5ef50ae2eae..db4e409fa2d4d9 100644 --- a/lib/util/yaml.ts +++ b/lib/util/yaml.ts @@ -1,5 +1,6 @@ import type { CreateNodeOptions, + Document, DocumentOptions, ParseOptions, SchemaOptions, From e38151c885adb126061612e916e319be78fa127a Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 22:06:34 +0200 Subject: [PATCH 05/46] refactor: move updatePnpmCatalogDependency to its own module --- .../manager/npm/update/dependency/index.ts | 98 +------------------ .../manager/npm/update/dependency/pnpm.ts | 94 ++++++++++++++++++ 2 files changed, 95 insertions(+), 97 deletions(-) create mode 100644 lib/modules/manager/npm/update/dependency/pnpm.ts diff --git a/lib/modules/manager/npm/update/dependency/index.ts b/lib/modules/manager/npm/update/dependency/index.ts index 9bb7deb2b396e6..4c4a28aa7e4a19 100644 --- a/lib/modules/manager/npm/update/dependency/index.ts +++ b/lib/modules/manager/npm/update/dependency/index.ts @@ -1,12 +1,9 @@ import is from '@sindresorhus/is'; import { dequal } from 'dequal'; -import { parseDocument } from 'yaml'; import { logger } from '../../../../../logger'; import { escapeRegExp, regEx } from '../../../../../util/regex'; import { matchAt, replaceAt } from '../../../../../util/string'; -import { parseSingleYaml } from '../../../../../util/yaml'; import type { UpdateDependencyConfig, Upgrade } from '../../../types'; -import { pnpmCatalogsSchema } from '../../extract/pnpm'; import type { DependenciesMeta, NpmPackage, @@ -14,6 +11,7 @@ import type { RecursiveOverride, } from '../../extract/types'; import type { NpmDepType, NpmManagerData } from '../../types'; +import { updatePnpmCatalogDependency } from './pnpm'; function renameObjKey( oldObj: DependenciesMeta, @@ -31,12 +29,6 @@ function renameObjKey( }, {} as DependenciesMeta); } -// TODO(fpapado): implement this -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function replaceYaml(): string { - return 'TODO'; -} - function replaceAsString( parsedContents: NpmPackage, fileContent: string, @@ -120,94 +112,6 @@ function replaceAsString( throw new Error(); } -// TODO(fpapado): move this somewhere else, e.g. npm/update/dependency/pnpm -function updatePnpmCatalogDependency({ - fileContent, - upgrade, -}: UpdateDependencyConfig): string | null { - const { depType, managerData, depName } = upgrade; - - const catalogName = managerData?.catalogName; - - if (!is.string(catalogName)) { - logger.error( - 'No catalogName was found; this is likely an extractoin error.', - ); - return null; - } - - let { newValue } = upgrade; - - // Handle git tags or digests - if (upgrade.currentRawValue) { - if (upgrade.currentDigest) { - logger.debug('Updating package.json git digest'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentDigest, - // TODO #22198 - - upgrade.newDigest!.substring(0, upgrade.currentDigest.length), - ); - } else { - logger.debug('Updating package.json git version tag'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentValue, - upgrade.newValue, - ); - } - } - - // Handle aliases - if (upgrade.npmPackageAlias) { - newValue = `npm:${upgrade.packageName}@${newValue}`; - } - - logger.info( - `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, - ); - - // Save the old version - let parsedContents; - - try { - // TODO: we should move pnpmCatalogsSchema around to a common/shared location in the npm manager - parsedContents = parseSingleYaml(fileContent, { - customSchema: pnpmCatalogsSchema, - }); - } catch (err) { - logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); - return null; - } - - const oldVersion = - catalogName === 'default' - ? parsedContents.catalog?.[depName!] - : parsedContents.catalogs?.[catalogName]?.[depName!]; - - if (oldVersion === newValue) { - logger.trace('Version is already updated'); - return fileContent; - } - - // TODO: Consider separating `parseSingleYaml` and - // `parseSingleYamlDocument`/`yamlDocumentToJS`, so that we can keep the unified API - const document = parseDocument(fileContent, { - uniqueKeys: false, - strict: false, - }); - - if (catalogName === 'default') { - // TODO: We should check whether `catalogs\n:default:\npkg: 1.0.0` is valid, - // because in that case we need to know whether it was set in `catalog` or - // `catalogs\ndefault` - document.setIn(['catalog', depName], newValue); - } else { - document.setIn(['catalogs', catalogName, depName], newValue); - } - - return document.toString(); -} - export function updateDependency({ fileContent, upgrade, diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts new file mode 100644 index 00000000000000..3c104d4b53a7e0 --- /dev/null +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -0,0 +1,94 @@ +import is from '@sindresorhus/is'; +import { parseDocument } from 'yaml'; +import { logger } from '../../../../../logger'; +import { parseSingleYaml } from '../../../../../util/yaml'; +import type { UpdateDependencyConfig } from '../../../types'; +import { pnpmCatalogsSchema } from '../../extract/pnpm'; + +// TODO(fpapado): move this somewhere else, e.g. npm/update/dependency/pnpm +export function updatePnpmCatalogDependency({ + fileContent, + upgrade, +}: UpdateDependencyConfig): string | null { + const { depType, managerData, depName } = upgrade; + + const catalogName = managerData?.catalogName; + + if (!is.string(catalogName)) { + logger.error( + 'No catalogName was found; this is likely an extractoin error.', + ); + return null; + } + + let { newValue } = upgrade; + + // Handle git tags or digests + if (upgrade.currentRawValue) { + if (upgrade.currentDigest) { + logger.debug('Updating package.json git digest'); + newValue = upgrade.currentRawValue.replace( + upgrade.currentDigest, + // TODO #22198 + + upgrade.newDigest!.substring(0, upgrade.currentDigest.length), + ); + } else { + logger.debug('Updating package.json git version tag'); + newValue = upgrade.currentRawValue.replace( + upgrade.currentValue, + upgrade.newValue, + ); + } + } + + // Handle aliases + if (upgrade.npmPackageAlias) { + newValue = `npm:${upgrade.packageName}@${newValue}`; + } + + logger.info( + `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, + ); + + // Save the old version + let parsedContents; + + try { + // TODO: we should move pnpmCatalogsSchema around to a common/shared location in the npm manager + parsedContents = parseSingleYaml(fileContent, { + customSchema: pnpmCatalogsSchema, + }); + } catch (err) { + logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); + return null; + } + + const oldVersion = + catalogName === 'default' + ? parsedContents.catalog?.[depName!] + : parsedContents.catalogs?.[catalogName]?.[depName!]; + + if (oldVersion === newValue) { + logger.trace('Version is already updated'); + return fileContent; + } + + // TODO: Consider separating `parseSingleYaml` and + // `parseSingleYamlDocument`/`yamlDocumentToJS`, so that we can keep the unified API + const document = parseDocument(fileContent, { + uniqueKeys: false, + strict: false, + }); + + if (catalogName === 'default') { + // TODO: We should check whether `catalogs\n:default:\npkg: 1.0.0` is valid, + // because in that case we need to know whether it was set in `catalog` or + // `catalogs\ndefault` + document.setIn(['catalog', depName], newValue); + } else { + document.setIn(['catalogs', catalogName, depName], newValue); + } + + return document.toString(); +} From 437ad816b839978ffaa1e309d01d5e9b9d2773a9 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 22:35:32 +0200 Subject: [PATCH 06/46] refactor(npm): move git and npm alias resolution to common module --- .../manager/npm/update/dependency/common.ts | 33 +++++++++++++++++++ .../manager/npm/update/dependency/index.ts | 22 ++----------- .../manager/npm/update/dependency/pnpm.ts | 27 ++------------- 3 files changed, 39 insertions(+), 43 deletions(-) create mode 100644 lib/modules/manager/npm/update/dependency/common.ts diff --git a/lib/modules/manager/npm/update/dependency/common.ts b/lib/modules/manager/npm/update/dependency/common.ts new file mode 100644 index 00000000000000..b61a459a4f53fd --- /dev/null +++ b/lib/modules/manager/npm/update/dependency/common.ts @@ -0,0 +1,33 @@ +import { logger } from '../../../../../logger'; +import type { Upgrade } from '../../../types'; + +export function getNewGitValue(upgrade: Upgrade): string | undefined { + if (!upgrade.currentRawValue) { + return; + } + if (upgrade.currentDigest) { + logger.debug('Updating git digest'); + return upgrade.currentRawValue.replace( + upgrade.currentDigest, + // TODO #22198 + + upgrade.newDigest!.substring(0, upgrade.currentDigest.length), + ); + } else { + logger.debug('Updating git version tag'); + return upgrade.currentRawValue.replace( + upgrade.currentValue, + upgrade.newValue, + ); + } +} + +export function getNewNpmAliasValue( + value: string | undefined, + upgrade: Upgrade, +): string | undefined { + if (!upgrade.npmPackageAlias) { + return; + } + return `npm:${upgrade.packageName}@${value}`; +} diff --git a/lib/modules/manager/npm/update/dependency/index.ts b/lib/modules/manager/npm/update/dependency/index.ts index 4c4a28aa7e4a19..71efb12d7b5852 100644 --- a/lib/modules/manager/npm/update/dependency/index.ts +++ b/lib/modules/manager/npm/update/dependency/index.ts @@ -11,6 +11,7 @@ import type { RecursiveOverride, } from '../../extract/types'; import type { NpmDepType, NpmManagerData } from '../../types'; +import { getNewGitValue, getNewNpmAliasValue } from './common'; import { updatePnpmCatalogDependency } from './pnpm'; function renameObjKey( @@ -123,26 +124,9 @@ export function updateDependency({ const { depType, managerData } = upgrade; const depName: string = managerData?.key || upgrade.depName; let { newValue } = upgrade; - if (upgrade.currentRawValue) { - if (upgrade.currentDigest) { - logger.debug('Updating package.json git digest'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentDigest, - // TODO #22198 - upgrade.newDigest!.substring(0, upgrade.currentDigest.length), - ); - } else { - logger.debug('Updating package.json git version tag'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentValue, - upgrade.newValue, - ); - } - } - if (upgrade.npmPackageAlias) { - newValue = `npm:${upgrade.packageName}@${newValue}`; - } + newValue = getNewGitValue(upgrade) ?? newValue; + newValue = getNewNpmAliasValue(newValue, upgrade) ?? newValue; logger.debug(`npm.updateDependency(): ${depType}.${depName} = ${newValue}`); try { diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 3c104d4b53a7e0..210719c8eea3bd 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -4,8 +4,8 @@ import { logger } from '../../../../../logger'; import { parseSingleYaml } from '../../../../../util/yaml'; import type { UpdateDependencyConfig } from '../../../types'; import { pnpmCatalogsSchema } from '../../extract/pnpm'; +import { getNewGitValue, getNewNpmAliasValue } from './common'; -// TODO(fpapado): move this somewhere else, e.g. npm/update/dependency/pnpm export function updatePnpmCatalogDependency({ fileContent, upgrade, @@ -23,29 +23,8 @@ export function updatePnpmCatalogDependency({ let { newValue } = upgrade; - // Handle git tags or digests - if (upgrade.currentRawValue) { - if (upgrade.currentDigest) { - logger.debug('Updating package.json git digest'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentDigest, - // TODO #22198 - - upgrade.newDigest!.substring(0, upgrade.currentDigest.length), - ); - } else { - logger.debug('Updating package.json git version tag'); - newValue = upgrade.currentRawValue.replace( - upgrade.currentValue, - upgrade.newValue, - ); - } - } - - // Handle aliases - if (upgrade.npmPackageAlias) { - newValue = `npm:${upgrade.packageName}@${newValue}`; - } + newValue = getNewGitValue(upgrade) ?? newValue; + newValue = getNewNpmAliasValue(newValue, upgrade) ?? newValue; logger.info( `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, From 5c38828ed1ed54fc367a20a539c02efc40204b64 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 22:53:20 +0200 Subject: [PATCH 07/46] chore: small test and typechecking fixups --- lib/modules/manager/api.ts | 1 - lib/modules/manager/npm/extract/pnpm.spec.ts | 8 ++++---- lib/util/yaml.ts | 1 - 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/modules/manager/api.ts b/lib/modules/manager/api.ts index c4219509abe229..573fcf95375e93 100644 --- a/lib/modules/manager/api.ts +++ b/lib/modules/manager/api.ts @@ -78,7 +78,6 @@ import * as pipCompile from './pip-compile'; import * as pip_requirements from './pip_requirements'; import * as pip_setup from './pip_setup'; import * as pipenv from './pipenv'; -import * as pnpmCatalog from './pnpm-catalog'; import * as poetry from './poetry'; import * as preCommit from './pre-commit'; import * as pub from './pub'; diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 2f82d2a76ed209..2f85c7b3de7b9e 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -323,8 +323,8 @@ describe('modules/manager/npm/extract/pnpm', () => { currentValue: '18.3.0', datasource: 'npm', depName: 'react', - depType: 'catalogDependency', - prettyDepType: 'catalogDependency', + depType: 'pnpm.catalog', + prettyDepType: 'pnpm.catalog', managerData: { catalogName: 'default', }, @@ -333,8 +333,8 @@ describe('modules/manager/npm/extract/pnpm', () => { currentValue: '17.0.2', datasource: 'npm', depName: 'react', - depType: 'catalogDependency', - prettyDepType: 'catalogDependency', + depType: 'pnpm.catalog', + prettyDepType: 'pnpm.catalog', managerData: { catalogName: 'react17', }, diff --git a/lib/util/yaml.ts b/lib/util/yaml.ts index db4e409fa2d4d9..e8d5ef50ae2eae 100644 --- a/lib/util/yaml.ts +++ b/lib/util/yaml.ts @@ -1,6 +1,5 @@ import type { CreateNodeOptions, - Document, DocumentOptions, ParseOptions, SchemaOptions, From 6de942c92b3ebfcc693a2446c78748e4dc11f4ed Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 23:17:03 +0200 Subject: [PATCH 08/46] refactor(npm,utils): split out parseSingleYamlDocument This keeps YAML parsing centralised, while allowing use of the Document represenation, in addition to the JS one. --- .../manager/npm/update/dependency/pnpm.ts | 21 ++++------ lib/util/yaml.ts | 39 +++++++++++++++---- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 210719c8eea3bd..03f79782c0e402 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -1,7 +1,6 @@ import is from '@sindresorhus/is'; -import { parseDocument } from 'yaml'; import { logger } from '../../../../../logger'; -import { parseSingleYaml } from '../../../../../util/yaml'; +import { parseSingleYamlDocument } from '../../../../../util/yaml'; import type { UpdateDependencyConfig } from '../../../types'; import { pnpmCatalogsSchema } from '../../extract/pnpm'; import { getNewGitValue, getNewNpmAliasValue } from './common'; @@ -30,19 +29,20 @@ export function updatePnpmCatalogDependency({ `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, ); - // Save the old version + let document; let parsedContents; try { - // TODO: we should move pnpmCatalogsSchema around to a common/shared location in the npm manager - parsedContents = parseSingleYaml(fileContent, { - customSchema: pnpmCatalogsSchema, - }); + // TODO: we should move pnpmCatalogsSchema around to a common/shared + // location in the npm manager + document = parseSingleYamlDocument(fileContent); + parsedContents = pnpmCatalogsSchema.parse(document.toJS()); } catch (err) { logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); return null; } + // Save the old version const oldVersion = catalogName === 'default' ? parsedContents.catalog?.[depName!] @@ -53,13 +53,6 @@ export function updatePnpmCatalogDependency({ return fileContent; } - // TODO: Consider separating `parseSingleYaml` and - // `parseSingleYamlDocument`/`yamlDocumentToJS`, so that we can keep the unified API - const document = parseDocument(fileContent, { - uniqueKeys: false, - strict: false, - }); - if (catalogName === 'default') { // TODO: We should check whether `catalogs\n:default:\npkg: 1.0.0` is valid, // because in that case we need to know whether it was set in `catalog` or diff --git a/lib/util/yaml.ts b/lib/util/yaml.ts index e8d5ef50ae2eae..c0de0ede60feda 100644 --- a/lib/util/yaml.ts +++ b/lib/util/yaml.ts @@ -1,5 +1,6 @@ import type { CreateNodeOptions, + Document, DocumentOptions, ParseOptions, SchemaOptions, @@ -20,6 +21,13 @@ interface YamlOptions< removeTemplates?: boolean; } +interface YamlParseDocumentOptions + extends ParseOptions, + DocumentOptions, + SchemaOptions { + removeTemplates?: boolean; +} + interface YamlOptionsMultiple< ResT = unknown, Schema extends ZodType = ZodType, @@ -117,6 +125,29 @@ export function parseSingleYaml( content: string, options?: YamlOptions, ): ResT { + const rawDocument = parseSingleYamlDocument(content, options); + + const document = rawDocument.toJS({ maxAliasCount: 10000 }); + const schema = options?.customSchema; + if (!schema) { + return document as ResT; + } + + return schema.parse(document); +} + +/** + * Parse a YAML string into a Document representation. + * + * Only a single document is supported. + * + * @param content + * @param options + */ +export function parseSingleYamlDocument( + content: string, + options?: YamlParseDocumentOptions, +): Document { const massagedContent = massageContent(content, options); const rawDocument = parseDocument( massagedContent, @@ -127,13 +158,7 @@ export function parseSingleYaml( throw new AggregateError(rawDocument.errors, 'Failed to parse YAML file'); } - const document = rawDocument.toJS({ maxAliasCount: 10000 }); - const schema = options?.customSchema; - if (!schema) { - return document as ResT; - } - - return schema.parse(document); + return rawDocument; } export function dump(obj: any, opts?: DumpOptions): string { From 6418f78bdf47a26646f95a0674792205aeea77bb Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 23:39:12 +0200 Subject: [PATCH 09/46] fix(npm): set either implicit or named default catalog --- lib/modules/manager/npm/update/dependency/pnpm.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 03f79782c0e402..17e6c6a49b6545 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -42,9 +42,15 @@ export function updatePnpmCatalogDependency({ return null; } + // In pnpm-workspace.yaml, the default catalog can be either `catalog` or + // `catalog.default`, but not both (pnpm throws outright with a config error). + // Thus, we must check which entry is being used, to reference it from the + // right place. + const usesImplicitDefaultCatalog = parsedContents.catalog !== undefined; + // Save the old version const oldVersion = - catalogName === 'default' + catalogName === 'default' && usesImplicitDefaultCatalog ? parsedContents.catalog?.[depName!] : parsedContents.catalogs?.[catalogName]?.[depName!]; @@ -53,10 +59,7 @@ export function updatePnpmCatalogDependency({ return fileContent; } - if (catalogName === 'default') { - // TODO: We should check whether `catalogs\n:default:\npkg: 1.0.0` is valid, - // because in that case we need to know whether it was set in `catalog` or - // `catalogs\ndefault` + if (catalogName === 'default' && usesImplicitDefaultCatalog) { document.setIn(['catalog', depName], newValue); } else { document.setIn(['catalogs', catalogName, depName], newValue); From a77ba8449108bbc38b374cb0a3ac8a4ad386bd71 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 23:44:58 +0200 Subject: [PATCH 10/46] chore: remove TODO --- lib/modules/manager/npm/update/dependency/pnpm.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 17e6c6a49b6545..e063c7bee5645b 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -33,8 +33,6 @@ export function updatePnpmCatalogDependency({ let parsedContents; try { - // TODO: we should move pnpmCatalogsSchema around to a common/shared - // location in the npm manager document = parseSingleYamlDocument(fileContent); parsedContents = pnpmCatalogsSchema.parse(document.toJS()); } catch (err) { From 9204b3a4e4c304e9654a079a4cc87d63b74a4c33 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Wed, 1 Jan 2025 23:54:11 +0200 Subject: [PATCH 11/46] chore(npm): tidy up TODOs for review --- lib/modules/manager/npm/extract/index.ts | 3 ++- lib/modules/manager/npm/extract/pnpm.ts | 23 +++++++++++++---------- lib/modules/manager/npm/index.ts | 2 +- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index 18cc86c024f003..6faaad19ea8e25 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -230,7 +230,8 @@ export async function extractAllPackageFiles( const content = await readLocalFile(packageFile, 'utf8'); // istanbul ignore else if (content) { - // TODO(fpapado): consider whether to do this here, or in the postExtract step + // TODO(fpapado): for PR dicussion, consider this vs. a post hook, where + // we look for pnpm-workspace.yaml in the siblings if (packageFile === 'pnpm-workspace.yaml') { const deps = extractPnpmWorkspaceFile(content, packageFile); if (deps) { diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index f7fa7b51e3c95d..ff5337c191b1bf 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -281,12 +281,20 @@ function extractPnpmCatalogDeps( const depName = parseDepName(CATALOG_DEPENDENCY, key); let dep: PackageDependency = { depType: CATALOG_DEPENDENCY, - // TODO(fpapado): consider how users might be able to match on specific - // catalogs, such as catalog.default.react or catalog.react17.react + // TODO(fpapado): for PR discussion, consider how users might be able to + // match on specific catalogs for their config. + // + // For example, we could change depType to `pnpm.catalog.${string}`, so + // that users can match use `{matchDepTypes: ["pnpm.catalog.default"]}`, + // `{matchDepTypes: ["pnpm.catalog.react17"]}` and so on. + // + // Another option would be to mess with depName/packageName. + // + // Is there precedence for something similar? depName, managerData: { - // we assign the name of the catalog, in order to know what fields to - // update later on + // We assign the name of the catalog, in order to know which fields to + // update later on. catalogName: catalog.name, }, }; @@ -297,12 +305,7 @@ function extractPnpmCatalogDeps( // TODO: fix type #22198 dep = { ...dep, - ...extractDependency( - CATALOG_DEPENDENCY, - depName, - // FIXME(fpapado): small crime with `val!` - val!, - ), + ...extractDependency(CATALOG_DEPENDENCY, depName, val!), prettyDepType: CATALOG_DEPENDENCY, }; dep.prettyDepType = CATALOG_DEPENDENCY; diff --git a/lib/modules/manager/npm/index.ts b/lib/modules/manager/npm/index.ts index 0834cb36d966ad..e8550072f49c72 100644 --- a/lib/modules/manager/npm/index.ts +++ b/lib/modules/manager/npm/index.ts @@ -20,7 +20,7 @@ export const url = 'https://docs.npmjs.com'; export const categories: Category[] = ['js']; export const defaultConfig = { - // TODO: Consider this vs. a post hook + // TODO(fpapado): for PR dicussion, consider this vs. a post hook fileMatch: ['(^|/)package\\.json$', '(^|/)pnpm-workspace\\.yaml$'], digest: { prBodyDefinitions: { From 5d399170a78440d7c7b87ad6a5322f9df716cdc1 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Thu, 2 Jan 2025 10:59:52 +0200 Subject: [PATCH 12/46] test(npm): add tests for pnpm catalog updates --- .../npm/update/dependency/pnpm.spec.ts | 204 ++++++++++++++++++ .../manager/npm/update/dependency/pnpm.ts | 15 +- 2 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 lib/modules/manager/npm/update/dependency/pnpm.spec.ts diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts new file mode 100644 index 00000000000000..e82b83f1837371 --- /dev/null +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -0,0 +1,204 @@ +import { codeBlock } from 'common-tags'; +import * as npmUpdater from '../..'; + +/** + * Per the YAML spec, a document ends with a newline. The 'yaml' library always + * uses that when serialising, but `codeBlock` strips the last indentation. This + * helper makes assertions simpler. + */ +function yamlCodeBlock( + literals: TemplateStringsArray, + ...placeholders: any[] +): string { + return codeBlock(literals, placeholders) + '\n'; +} + +describe('modules/manager/npm/update/dependency/pnpm', () => { + it('handles implicit default catalog dependency', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 19.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('handles explicit default catalog dependency', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalogs: + default: + react: 18.3.1 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalogs: + default: + react: 19.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('handles explicit named catalog dependency', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'react17', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 + + catalogs: + react17: + react: 17.0.0 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 + + catalogs: + react17: + react: 19.0.0 + + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it.failing('replaces package', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'config', + newName: 'abc', + newValue: '2.0.0', + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + config: 1.21.0 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + abc: 2.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('returns null if the dependency is not present in the target catalog', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react-not', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toBeNull(); + }); + + it('returns null if catalogs are missing', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toBeNull(); + }); + + it('returns null if empty file', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const testContent = npmUpdater.updateDependency({ + fileContent: null as never, + upgrade, + }); + expect(testContent).toBeNull(); + }); +}); diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index e063c7bee5645b..bec571a93e13d0 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -1,4 +1,5 @@ import is from '@sindresorhus/is'; +import { stringify } from 'yaml'; import { logger } from '../../../../../logger'; import { parseSingleYamlDocument } from '../../../../../util/yaml'; import type { UpdateDependencyConfig } from '../../../types'; @@ -15,7 +16,7 @@ export function updatePnpmCatalogDependency({ if (!is.string(catalogName)) { logger.error( - 'No catalogName was found; this is likely an extractoin error.', + 'No catalogName was found; this is likely an extraction error.', ); return null; } @@ -25,7 +26,7 @@ export function updatePnpmCatalogDependency({ newValue = getNewGitValue(upgrade) ?? newValue; newValue = getNewNpmAliasValue(newValue, upgrade) ?? newValue; - logger.info( + logger.debug( `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, ); @@ -57,11 +58,19 @@ export function updatePnpmCatalogDependency({ return fileContent; } + // TODO: handle depName === oldValue + // The old value is the name of the dependency itself + + if (oldVersion === undefined) { + // There is some subtlety here + return null; + } + if (catalogName === 'default' && usesImplicitDefaultCatalog) { document.setIn(['catalog', depName], newValue); } else { document.setIn(['catalogs', catalogName, depName], newValue); } - return document.toString(); + return stringify(document); } From 2d9c8bc128d87bcdc18287bb6b9c762b132f509d Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Thu, 2 Jan 2025 11:26:33 +0200 Subject: [PATCH 13/46] feat(npm): implement replacement for pnpm catalog updates --- .../npm/update/dependency/pnpm.spec.ts | 5 ++- .../manager/npm/update/dependency/pnpm.ts | 44 +++++++++++++++---- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index e82b83f1837371..b18ebe5baa276e 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -115,12 +115,15 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { expect(testContent).toEqual(expected); }); - it.failing('replaces package', () => { + it('replaces package', () => { const upgrade = { depType: 'pnpm.catalog', depName: 'config', newName: 'abc', newValue: '2.0.0', + managerData: { + catalogName: 'default', + }, }; const pnpmWorkspaceYaml = yamlCodeBlock` packages: diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index bec571a93e13d0..c7441a9e42e206 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -58,19 +58,47 @@ export function updatePnpmCatalogDependency({ return fileContent; } - // TODO: handle depName === oldValue - // The old value is the name of the dependency itself + // Update the value + const path = getDepPath({ + depName: depName!, + catalogName, + usesImplicitDefaultCatalog, + }); - if (oldVersion === undefined) { - // There is some subtlety here + if (!document.hasIn(path)) { return null; } - if (catalogName === 'default' && usesImplicitDefaultCatalog) { - document.setIn(['catalog', depName], newValue); - } else { - document.setIn(['catalogs', catalogName, depName], newValue); + document.setIn(path, newValue); + + // Update the name, for replacements + if (upgrade.newName) { + const newPath = getDepPath({ + depName: upgrade.newName, + catalogName, + usesImplicitDefaultCatalog, + }); + const oldValue = document.getIn(path); + + document.deleteIn(path); + document.setIn(newPath, oldValue); } return stringify(document); } + +function getDepPath({ + catalogName, + depName, + usesImplicitDefaultCatalog, +}: { + usesImplicitDefaultCatalog: boolean; + catalogName: string; + depName: string; +}): string[] { + if (catalogName === 'default' && usesImplicitDefaultCatalog) { + return ['catalog', depName]; + } else { + return ['catalogs', catalogName, depName]; + } +} From 03e44e1196d0619a40d28db704e108542f90093b Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Thu, 2 Jan 2025 11:27:26 +0200 Subject: [PATCH 14/46] chore: remove done TODO --- lib/modules/manager/npm/extract/pnpm.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index ff5337c191b1bf..244f051b74de76 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -338,5 +338,3 @@ function parsePnpmCatalogs(content: string): Array { })), ]; } - -// TODO: When extracting, use {depType: 'catalogDependency', depName: 'name.packageName'} From d62d7d12b430630547daf48398662d177043a0ef Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Thu, 2 Jan 2025 18:23:42 +0200 Subject: [PATCH 15/46] test(npm): mirror specifier tests to catalog update --- .../npm/update/dependency/pnpm.spec.ts | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index b18ebe5baa276e..d6768a08e68600 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -146,6 +146,135 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { expect(testContent).toEqual(expected); }); + it('replaces a github dependency value', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'gulp', + currentValue: 'v4.0.0-alpha.2', + currentRawValue: 'gulpjs/gulp#v4.0.0-alpha.2', + newValue: 'v4.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + gulp: gulpjs/gulp#v4.0.0-alpha.2 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + gulp: gulpjs/gulp#v4.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('replaces a npm package alias', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'hapi', + npmPackageAlias: true, + packageName: '@hapi/hapi', + currentValue: '18.3.0', + newValue: '18.3.1', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + hapi: npm:@hapi/hapi@18.3.0 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + hapi: npm:@hapi/hapi@18.3.1 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('replaces a github short hash', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'gulp', + currentDigest: 'abcdef7', + currentRawValue: 'gulpjs/gulp#abcdef7', + newDigest: '0000000000111111111122222222223333333333', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + gulp: gulpjs/gulp#abcdef7 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + gulp: gulpjs/gulp#0000000 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + + it('replaces a github fully specified version', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'n', + currentValue: 'v1.0.0', + currentRawValue: 'git+https://github.com/owner/n#v1.0.0', + newValue: 'v1.1.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + n: git+https://github.com/owner/n#v1.0.0 + `; + const expected = yamlCodeBlock` + packages: + - pkg-a + + catalog: + n: git+https://github.com/owner/n#v1.1.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(expected); + }); + it('returns null if the dependency is not present in the target catalog', () => { const upgrade = { depType: 'pnpm.catalog', From 2f56199bfd6f95d4cf2faff8873726f9d69ef8ab Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Thu, 2 Jan 2025 18:34:51 +0200 Subject: [PATCH 16/46] chore: tidy up pnpm extraction test and TODO --- lib/modules/manager/npm/extract/pnpm.spec.ts | 18 +++++++++--------- lib/modules/manager/npm/extract/pnpm.ts | 1 - 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 2f85c7b3de7b9e..41b245e8a0af61 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -296,9 +296,9 @@ describe('modules/manager/npm/extract/pnpm', () => { expect( extractPnpmWorkspaceFile( codeBlock` - catalog: - catalogs: - `, + catalog: + catalogs: + `, 'pnpm-workspace.yaml', ), ).toBeNull(); @@ -308,13 +308,13 @@ describe('modules/manager/npm/extract/pnpm', () => { expect( extractPnpmWorkspaceFile( codeBlock` - catalog: - react: 18.3.0 + catalog: + react: 18.3.0 - catalogs: - react17: - react: 17.0.2 - `, + catalogs: + react17: + react: 17.0.2 + `, 'pnpm-workspace.yaml', ), ).toMatchObject({ diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 244f051b74de76..8b02a689edbee8 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -242,7 +242,6 @@ type PnpmCatalog = { name: string; dependencies: NpmPackageDependency }; export function extractPnpmWorkspaceFile( content: string, packageFile: string, - // TODO: consider config: ExtractConfig here ): PackageFile | null { logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); From 59f0a83ca491c84b96ad8a5feb6c40d41a09c3d4 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Fri, 10 Jan 2025 14:10:43 +0200 Subject: [PATCH 17/46] Batch apply suggestions from code review Co-authored-by: Sebastian Poxhofer --- lib/modules/manager/npm/extract/pnpm.ts | 29 ++++++++++++------- .../manager/npm/update/dependency/common.ts | 8 ++--- .../npm/update/dependency/pnpm.spec.ts | 13 ++++----- .../manager/npm/update/dependency/pnpm.ts | 2 +- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 8b02a689edbee8..f009376abc766c 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -245,9 +245,9 @@ export function extractPnpmWorkspaceFile( ): PackageFile | null { logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); - let pnpmCatalogs: Array; + const pnpmCatalogs: PnpmCatalog[] = []; try { - pnpmCatalogs = parsePnpmCatalogs(content); + pnpmCatalogs.push(...parsePnpmCatalogs(content)); } catch { logger.debug({ packageFile }, `Invalid pnpm workspace YAML.`); return null; @@ -256,11 +256,9 @@ export function extractPnpmWorkspaceFile( const extracted = extractPnpmCatalogDeps(pnpmCatalogs); if (!extracted) { - logger.debug({ packageFile }, 'No dependencies found'); return null; } - logger.debug(extracted, 'Extracted catalog dependencies.'); return { ...extracted, @@ -269,7 +267,7 @@ export function extractPnpmWorkspaceFile( } function extractPnpmCatalogDeps( - catalogs: Array, + catalogs: PnpmCatalog[], ): PackageFileContent | null { const CATALOG_DEPENDENCY = 'pnpm.catalog'; @@ -322,18 +320,27 @@ export const pnpmCatalogsSchema = z.object({ catalogs: z.optional(z.record(z.record(z.string()))), }); -function parsePnpmCatalogs(content: string): Array { +function parsePnpmCatalogs(content: string): PnpmCatalog[] { const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = parseSingleYaml(content, { customSchema: pnpmCatalogsSchema }); - return [ + const result = [ { name: 'default', dependencies: defaultCatalogDeps ?? {}, - }, - ...Object.entries(namedCatalogs ?? {}).map(([name, dependencies]) => ({ + } + ] + + if (!namedCatalogs) { + return result; + } + + for (const [name, dependencies] of Object.entries(namedCatalogs)) { + result.push({ name, dependencies, - })), - ]; + }); + } + + return result } diff --git a/lib/modules/manager/npm/update/dependency/common.ts b/lib/modules/manager/npm/update/dependency/common.ts index b61a459a4f53fd..92c9172677805d 100644 --- a/lib/modules/manager/npm/update/dependency/common.ts +++ b/lib/modules/manager/npm/update/dependency/common.ts @@ -1,9 +1,9 @@ import { logger } from '../../../../../logger'; import type { Upgrade } from '../../../types'; -export function getNewGitValue(upgrade: Upgrade): string | undefined { +export function getNewGitValue(upgrade: Upgrade): string | null { if (!upgrade.currentRawValue) { - return; + return null; } if (upgrade.currentDigest) { logger.debug('Updating git digest'); @@ -25,9 +25,9 @@ export function getNewGitValue(upgrade: Upgrade): string | undefined { export function getNewNpmAliasValue( value: string | undefined, upgrade: Upgrade, -): string | undefined { +): string | null { if (!upgrade.npmPackageAlias) { - return; + return null; } return `npm:${upgrade.packageName}@${value}`; } diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index d6768a08e68600..bf9f7f9a4fa171 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -30,18 +30,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: react: 18.3.1 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: react: 19.0.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `;); }); it('handles explicit default catalog dependency', () => { diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index c7441a9e42e206..7b180d540746d1 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -26,7 +26,7 @@ export function updatePnpmCatalogDependency({ newValue = getNewGitValue(upgrade) ?? newValue; newValue = getNewNpmAliasValue(newValue, upgrade) ?? newValue; - logger.debug( + logger.trace( `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, ); From f61958ade9f167bd05f7233c1e2807a66882d25e Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Fri, 10 Jan 2025 14:15:09 +0200 Subject: [PATCH 18/46] chore: inline expectations in pnpm update spec --- .../npm/update/dependency/pnpm.spec.ts | 93 +++++++++---------- 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index bf9f7f9a4fa171..21fa8720632c08 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -40,7 +40,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: react: 19.0.0 - `;); + `); }); it('handles explicit default catalog dependency', () => { @@ -60,19 +60,18 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { default: react: 18.3.1 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalogs: default: react: 19.0.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('handles explicit named catalog dependency', () => { @@ -95,7 +94,11 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { react17: react: 17.0.0 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a @@ -106,12 +109,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { react17: react: 19.0.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('replaces package', () => { @@ -131,18 +129,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: config: 1.21.0 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: abc: 2.0.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('replaces a github dependency value', () => { @@ -163,18 +160,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: gulp: gulpjs/gulp#v4.0.0-alpha.2 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: gulp: gulpjs/gulp#v4.0.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('replaces a npm package alias', () => { @@ -196,18 +192,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: hapi: npm:@hapi/hapi@18.3.0 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: hapi: npm:@hapi/hapi@18.3.1 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('replaces a github short hash', () => { @@ -228,18 +223,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: gulp: gulpjs/gulp#abcdef7 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: gulp: gulpjs/gulp#0000000 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('replaces a github fully specified version', () => { @@ -260,18 +254,17 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalog: n: git+https://github.com/owner/n#v1.0.0 `; - const expected = yamlCodeBlock` + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` packages: - pkg-a catalog: n: git+https://github.com/owner/n#v1.1.0 - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(expected); + `); }); it('returns null if the dependency is not present in the target catalog', () => { From 2262d4b9c0597d12a4337230f2a659d65cab23e4 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 14:23:35 +0200 Subject: [PATCH 19/46] fix: use a CST for updating catalogs, in order to preserve formatting --- .../npm/update/dependency/pnpm.spec.ts | 250 ++++++++++++++++++ .../manager/npm/update/dependency/pnpm.ts | 103 ++++++-- 2 files changed, 336 insertions(+), 17 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 21fa8720632c08..26a0d7bb254161 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -325,4 +325,254 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { }); expect(testContent).toBeNull(); }); + + it('preserves formatting in flow style syntax', () => { + // Context: YAML has different representations for collections. Depending on + // the replacement method, the formatting of those blocks might change. It + // is an explicit goal to _not_ affect formatting, outside of changing the + // intended version. + // + // TODO(fpapado): consider enumerating the options + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'react17', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalogs: { + react17: { + react: 18.3.1 + } + } + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalogs: { + react17: { + react: 19.0.0 + } + } + `); + }); + + it('preserves literal whitespace', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: 19.0.0 + `); + }); + + it('preserves single quote style', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: '18.3.1' + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: '19.0.0' + `); + }); + + it('preserves double quote style', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: "18.3.1" + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: "19.0.0" + `); + }); + + it('preserves anchors, replacing only the value', () => { + // At the time of writing, this pattern is the current way to sync + // dependencies in catalogs. + // @see https://github.com/pnpm/pnpm/issues/8245#issuecomment-2371335323 + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react 18.3.1 + react-dom: *react + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react 19.0.0 + react-dom: *react + `); + }); + + it('preserves whitespace with anchors', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react 18.3.1 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react 19.0.0 + `); + }); + + it('preserves quotation style with anchors', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react "18.3.1" + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(yamlCodeBlock` + packages: + - pkg-a + + catalog: + react: &react "19.0.0" + `); + }); + + it('does not replace aliases', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + // In the general case, we do not know whether we should replace the anchor + // that an alias is resolved from. We leave this up to the user, e.g. via a + // Regex custom manager. + const pnpmWorkspaceYaml = yamlCodeBlock` + __deps: + react: &react 18.3.1 + + packages: + - pkg-a + + catalog: + react: *react + react-dom: *react + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toBeNull(); + }); }); diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 7b180d540746d1..245c7dd6c95147 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -1,7 +1,7 @@ import is from '@sindresorhus/is'; -import { stringify } from 'yaml'; +import { CST, Parser } from 'yaml'; import { logger } from '../../../../../logger'; -import { parseSingleYamlDocument } from '../../../../../util/yaml'; +import { parseSingleYaml } from '../../../../../util/yaml'; import type { UpdateDependencyConfig } from '../../../types'; import { pnpmCatalogsSchema } from '../../extract/pnpm'; import { getNewGitValue, getNewNpmAliasValue } from './common'; @@ -30,12 +30,21 @@ export function updatePnpmCatalogDependency({ `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, ); - let document; + let cstDocument: CST.Token; let parsedContents; try { - document = parseSingleYamlDocument(fileContent); - parsedContents = pnpmCatalogsSchema.parse(document.toJS()); + // In order to preserve the original formatting as much as possible, we use + // the CST directly. Using the AST (result of parseDocument) from 'yaml' + // does not guarantee that formatting would be the same after + // stringification. However, the CST is more annoying to query for certain + // values. Thus, we parse both as a CST and as a JS representation; the + // former for manipulation, and the latter for querying/validation. It is a bit + // wasteful, but it works. + cstDocument = Array.from(new Parser().parse(fileContent))[0]; + parsedContents = parseSingleYaml(fileContent, { + customSchema: pnpmCatalogsSchema, + }); } catch (err) { logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); return null; @@ -65,26 +74,86 @@ export function updatePnpmCatalogDependency({ usesImplicitDefaultCatalog, }); - if (!document.hasIn(path)) { + const doc = changeDependencyIn(cstDocument, path, { + newValue, + newName: upgrade.newName, + }); + + if (!doc) { + return null; + } + + return CST.stringify(doc); +} + +function changeDependencyIn( + doc: CST.Token, + path: string[], + { newName, newValue }: { newName?: string; newValue?: string }, +): CST.Document | null { + if (doc.type !== 'document') { + return null; + } + + const relevantNode = findInYamlCst(doc, path); + + if (!relevantNode) { return null; } - document.setIn(path, newValue); + if (newName) { + // TODO(fpapado): Think about this codepath a bit; can anchors / aliases appear in + // key positions? + if (!CST.isScalar(relevantNode.key)) { + return null; + } + CST.setScalarValue(relevantNode.key, newName); + } - // Update the name, for replacements - if (upgrade.newName) { - const newPath = getDepPath({ - depName: upgrade.newName, - catalogName, - usesImplicitDefaultCatalog, + if (newValue) { + // We only support scalar values when substituting. This explicitly avoids + // substituting aliases, since those can be resolved from a shared location, + // and replacing either the referrent anchor or the alias would be wrong in + // the general case. We leave this up to the user, e.g. via a Regex custom + // manager. + if (!CST.isScalar(relevantNode.value)) { + return null; + } + CST.setScalarValue(relevantNode.value, newValue); + } + + return doc; +} + +/** + * Find the collection item in a nested YAML collection, starting from a YAML root. + */ +function findInYamlCst( + root: CST.CollectionItem, + path: string[], +): CST.CollectionItem | null { + let currentNode = root; + + for (const segment of path) { + if (!CST.isCollection(currentNode?.value)) { + return null; + } + const newNode = currentNode.value.items.find((item) => { + if (!CST.isScalar(item.key)) { + return false; + } + const scalar = CST.resolveAsScalar(item.key); + return scalar.value === segment; }); - const oldValue = document.getIn(path); - document.deleteIn(path); - document.setIn(newPath, oldValue); + if (!newNode) { + return null; + } + + currentNode = newNode; } - return stringify(document); + return currentNode; } function getDepPath({ From 0a48dc75209884741c0b91cc75e207778f99a1e8 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 19:26:32 +0200 Subject: [PATCH 20/46] fix: avoid double-parsing in update --- .../npm/update/dependency/pnpm.spec.ts | 6 - .../manager/npm/update/dependency/pnpm.ts | 104 ++++++++---------- 2 files changed, 43 insertions(+), 67 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 26a0d7bb254161..9e32a8274e3cb3 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -327,12 +327,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { }); it('preserves formatting in flow style syntax', () => { - // Context: YAML has different representations for collections. Depending on - // the replacement method, the formatting of those blocks might change. It - // is an explicit goal to _not_ affect formatting, outside of changing the - // intended version. - // - // TODO(fpapado): consider enumerating the options const upgrade = { depType: 'pnpm.catalog', depName: 'react', diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 245c7dd6c95147..eba2094c3f711b 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -1,7 +1,7 @@ import is from '@sindresorhus/is'; -import { CST, Parser } from 'yaml'; +import type { Document } from 'yaml'; +import { CST, isCollection, isPair, isScalar, parseDocument } from 'yaml'; import { logger } from '../../../../../logger'; -import { parseSingleYaml } from '../../../../../util/yaml'; import type { UpdateDependencyConfig } from '../../../types'; import { pnpmCatalogsSchema } from '../../extract/pnpm'; import { getNewGitValue, getNewNpmAliasValue } from './common'; @@ -30,21 +30,18 @@ export function updatePnpmCatalogDependency({ `npm.updatePnpmCatalogDependency(): ${depType}:${managerData?.catalogName}.${depName} = ${newValue}`, ); - let cstDocument: CST.Token; + let document; let parsedContents; try { - // In order to preserve the original formatting as much as possible, we use - // the CST directly. Using the AST (result of parseDocument) from 'yaml' + // In order to preserve the original formatting as much as possible, we want + // manipulate the CST directly. Using the AST (the result of parseDocument) // does not guarantee that formatting would be the same after // stringification. However, the CST is more annoying to query for certain - // values. Thus, we parse both as a CST and as a JS representation; the - // former for manipulation, and the latter for querying/validation. It is a bit - // wasteful, but it works. - cstDocument = Array.from(new Parser().parse(fileContent))[0]; - parsedContents = parseSingleYaml(fileContent, { - customSchema: pnpmCatalogsSchema, - }); + // values. Thus, we use both an annotated AST and a JS representation; the + // former for manipulation, and the latter for querying/validation. + document = parseDocument(fileContent, { keepSourceTokens: true }); + parsedContents = pnpmCatalogsSchema.parse(document.toJS()); } catch (err) { logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); return null; @@ -52,11 +49,10 @@ export function updatePnpmCatalogDependency({ // In pnpm-workspace.yaml, the default catalog can be either `catalog` or // `catalog.default`, but not both (pnpm throws outright with a config error). - // Thus, we must check which entry is being used, to reference it from the - // right place. + // Thus, we must check which entry is being used, to reference it from / set + // it in the right place. const usesImplicitDefaultCatalog = parsedContents.catalog !== undefined; - // Save the old version const oldVersion = catalogName === 'default' && usesImplicitDefaultCatalog ? parsedContents.catalog?.[depName!] @@ -74,40 +70,57 @@ export function updatePnpmCatalogDependency({ usesImplicitDefaultCatalog, }); - const doc = changeDependencyIn(cstDocument, path, { + const modifiedDocument = changeDependencyIn(document, path, { newValue, newName: upgrade.newName, }); - if (!doc) { + if (!modifiedDocument) { + // Case where we are explicitly unable to substitute the key/value, for + // example if the value was an alias. return null; } - return CST.stringify(doc); + if (!modifiedDocument.contents?.srcToken) { + // This should not happen in practice, but we leave it to satisfy the types. + return null; + } + + return CST.stringify(modifiedDocument.contents.srcToken); } +/** + * Change the scalar name and/or value of a collection item in a YAML document, + * while keeping formatting consistent. Mutates the given document. + */ function changeDependencyIn( - doc: CST.Token, + document: Document, path: string[], { newName, newValue }: { newName?: string; newValue?: string }, -): CST.Document | null { - if (doc.type !== 'document') { +): Document | null { + const parentPath = path.slice(0, -1); + const relevantItemKey = path.at(-1); + + const parentNode = document.getIn(parentPath); + + if (!parentNode || !isCollection(parentNode)) { return null; } - const relevantNode = findInYamlCst(doc, path); + const relevantNode = parentNode.items.find( + (item) => + isPair(item) && isScalar(item.key) && item.key.value === relevantItemKey, + ); - if (!relevantNode) { + if (!relevantNode || !isPair(relevantNode)) { return null; } if (newName) { - // TODO(fpapado): Think about this codepath a bit; can anchors / aliases appear in - // key positions? - if (!CST.isScalar(relevantNode.key)) { + if (!CST.isScalar(relevantNode.srcToken?.key)) { return null; } - CST.setScalarValue(relevantNode.key, newName); + CST.setScalarValue(relevantNode.srcToken.key, newName); } if (newValue) { @@ -116,44 +129,13 @@ function changeDependencyIn( // and replacing either the referrent anchor or the alias would be wrong in // the general case. We leave this up to the user, e.g. via a Regex custom // manager. - if (!CST.isScalar(relevantNode.value)) { + if (!CST.isScalar(relevantNode.srcToken?.value)) { return null; } - CST.setScalarValue(relevantNode.value, newValue); - } - - return doc; -} - -/** - * Find the collection item in a nested YAML collection, starting from a YAML root. - */ -function findInYamlCst( - root: CST.CollectionItem, - path: string[], -): CST.CollectionItem | null { - let currentNode = root; - - for (const segment of path) { - if (!CST.isCollection(currentNode?.value)) { - return null; - } - const newNode = currentNode.value.items.find((item) => { - if (!CST.isScalar(item.key)) { - return false; - } - const scalar = CST.resolveAsScalar(item.key); - return scalar.value === segment; - }); - - if (!newNode) { - return null; - } - - currentNode = newNode; + CST.setScalarValue(relevantNode.srcToken.value, newValue); } - return currentNode; + return document; } function getDepPath({ From f3eea08b1bcda83340e4e1c677a3e0971e0d2d74 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 19:32:22 +0200 Subject: [PATCH 21/46] test: shuffle update tests, remove yamlCodeBlock Now that we use the CST for substitutions, we can accept inputs that do not end in newlines. Even though that is not per the yaml spec, it does mean that we leave any source files formatting intact. --- .../npm/update/dependency/pnpm.spec.ts | 142 ++++++++---------- 1 file changed, 63 insertions(+), 79 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 9e32a8274e3cb3..0a470b83a0cb37 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -1,18 +1,6 @@ import { codeBlock } from 'common-tags'; import * as npmUpdater from '../..'; -/** - * Per the YAML spec, a document ends with a newline. The 'yaml' library always - * uses that when serialising, but `codeBlock` strips the last indentation. This - * helper makes assertions simpler. - */ -function yamlCodeBlock( - literals: TemplateStringsArray, - ...placeholders: any[] -): string { - return codeBlock(literals, placeholders) + '\n'; -} - describe('modules/manager/npm/update/dependency/pnpm', () => { it('handles implicit default catalog dependency', () => { const upgrade = { @@ -23,7 +11,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -34,7 +22,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -52,7 +40,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -64,7 +52,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -83,7 +71,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'react17', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -98,7 +86,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -122,7 +110,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -133,7 +121,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -153,7 +141,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -164,7 +152,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -185,7 +173,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -196,7 +184,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -216,7 +204,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -227,7 +215,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -247,7 +235,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -258,7 +246,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -276,7 +264,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -299,7 +287,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a `; @@ -326,41 +314,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { expect(testContent).toBeNull(); }); - it('preserves formatting in flow style syntax', () => { - const upgrade = { - depType: 'pnpm.catalog', - depName: 'react', - newValue: '19.0.0', - managerData: { - catalogName: 'react17', - }, - }; - const pnpmWorkspaceYaml = yamlCodeBlock` - packages: - - pkg-a - - catalogs: { - react17: { - react: 18.3.1 - } - } - `; - const testContent = npmUpdater.updateDependency({ - fileContent: pnpmWorkspaceYaml, - upgrade, - }); - expect(testContent).toEqual(yamlCodeBlock` - packages: - - pkg-a - - catalogs: { - react17: { - react: 19.0.0 - } - } - `); - }); - it('preserves literal whitespace', () => { const upgrade = { depType: 'pnpm.catalog', @@ -370,7 +323,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -381,7 +334,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -399,7 +352,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -410,7 +363,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -428,7 +381,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -439,7 +392,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -449,7 +402,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { }); it('preserves anchors, replacing only the value', () => { - // At the time of writing, this pattern is the current way to sync + // At the time of writing, this pattern is the recommended way to sync // dependencies in catalogs. // @see https://github.com/pnpm/pnpm/issues/8245#issuecomment-2371335323 const upgrade = { @@ -460,7 +413,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -472,7 +425,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -491,7 +444,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -502,7 +455,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -520,7 +473,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { catalogName: 'default', }, }; - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` packages: - pkg-a @@ -531,7 +484,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { fileContent: pnpmWorkspaceYaml, upgrade, }); - expect(testContent).toEqual(yamlCodeBlock` + expect(testContent).toEqual(codeBlock` packages: - pkg-a @@ -540,6 +493,37 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { `); }); + it('preserves formatting in flow style syntax', () => { + const upgrade = { + depType: 'pnpm.catalog', + depName: 'react', + newValue: '19.0.0', + managerData: { + catalogName: 'default', + }, + }; + const pnpmWorkspaceYaml = codeBlock` + packages: + - pkg-a + + catalog: { + "react": "18.3.1" + } + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(codeBlock` + packages: + - pkg-a + + catalog: { + "react": "19.0.0" + } + `); + }); + it('does not replace aliases', () => { const upgrade = { depType: 'pnpm.catalog', @@ -552,7 +536,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { // In the general case, we do not know whether we should replace the anchor // that an alias is resolved from. We leave this up to the user, e.g. via a // Regex custom manager. - const pnpmWorkspaceYaml = yamlCodeBlock` + const pnpmWorkspaceYaml = codeBlock` __deps: react: &react 18.3.1 From 3312747a5b5162b59e0bc9c800846c8ca6d9f3fb Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 19:36:46 +0200 Subject: [PATCH 22/46] refactor: move PnpmCatalog interface to shared types.ts --- lib/modules/manager/npm/extract/pnpm.ts | 19 ++++--------------- lib/modules/manager/npm/extract/types.ts | 9 +++++++++ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index f009376abc766c..f6e0d259c44803 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -19,11 +19,7 @@ import type { import type { PnpmDependencySchema, PnpmLockFile } from '../post-update/types'; import type { NpmManagerData } from '../types'; import { extractDependency, parseDepName } from './common/dependency'; -import type { - LockFile, - NpmPackageDependency, - PnpmWorkspaceFile, -} from './types'; +import type { LockFile, PnpmCatalog, PnpmWorkspaceFile } from './types'; function isPnpmLockfile(obj: any): obj is PnpmLockFile { return is.plainObject(obj) && 'lockfileVersion' in obj; @@ -233,12 +229,6 @@ function getLockedDependencyVersions( return res; } -/** - * A pnpm catalog is either the default catalog (catalog:, catlog:default), or a - * named one (under catalogs:) - */ -type PnpmCatalog = { name: string; dependencies: NpmPackageDependency }; - export function extractPnpmWorkspaceFile( content: string, packageFile: string, @@ -259,7 +249,6 @@ export function extractPnpmWorkspaceFile( return null; } - return { ...extracted, packageFile, @@ -328,8 +317,8 @@ function parsePnpmCatalogs(content: string): PnpmCatalog[] { { name: 'default', dependencies: defaultCatalogDeps ?? {}, - } - ] + }, + ]; if (!namedCatalogs) { return result; @@ -342,5 +331,5 @@ function parsePnpmCatalogs(content: string): PnpmCatalog[] { }); } - return result + return result; } diff --git a/lib/modules/manager/npm/extract/types.ts b/lib/modules/manager/npm/extract/types.ts index 8b5c3ef7754236..0abffc5d5744a2 100644 --- a/lib/modules/manager/npm/extract/types.ts +++ b/lib/modules/manager/npm/extract/types.ts @@ -40,6 +40,15 @@ export interface PnpmWorkspaceFile { catalogs?: Partial>>>; } +/** + * A pnpm catalog is either the default catalog (catalog:, catalogs:default), or + * a named one (catalogs:) + */ +export interface PnpmCatalog { + name: string; + dependencies: NpmPackageDependency; +} + export type OverrideDependency = Record; export type RecursiveOverride = string | { [_: string]: RecursiveOverride }; From fbb488b311aae344356f5ecb2f4d33f197340c26 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 19:41:59 +0200 Subject: [PATCH 23/46] refactor: move pnpmCatalogsSchema to schema.ts --- lib/modules/manager/npm/extract/pnpm.ts | 9 ++------- lib/modules/manager/npm/schema.ts | 5 +++++ lib/modules/manager/npm/update/dependency/pnpm.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index f6e0d259c44803..90079901d0a02c 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -1,7 +1,6 @@ import is from '@sindresorhus/is'; import { findPackages } from 'find-packages'; import upath from 'upath'; -import { z } from 'zod'; import { GlobalConfig } from '../../../../config/global'; import { logger } from '../../../../logger'; import { @@ -17,6 +16,7 @@ import type { PackageFileContent, } from '../../types'; import type { PnpmDependencySchema, PnpmLockFile } from '../post-update/types'; +import { PnpmCatalogsSchema } from '../schema'; import type { NpmManagerData } from '../types'; import { extractDependency, parseDepName } from './common/dependency'; import type { LockFile, PnpmCatalog, PnpmWorkspaceFile } from './types'; @@ -304,14 +304,9 @@ function extractPnpmCatalogDeps( }; } -export const pnpmCatalogsSchema = z.object({ - catalog: z.optional(z.record(z.string())), - catalogs: z.optional(z.record(z.record(z.string()))), -}); - function parsePnpmCatalogs(content: string): PnpmCatalog[] { const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = - parseSingleYaml(content, { customSchema: pnpmCatalogsSchema }); + parseSingleYaml(content, { customSchema: PnpmCatalogsSchema }); const result = [ { diff --git a/lib/modules/manager/npm/schema.ts b/lib/modules/manager/npm/schema.ts index 79d986fea78507..8e264992dde75b 100644 --- a/lib/modules/manager/npm/schema.ts +++ b/lib/modules/manager/npm/schema.ts @@ -1,6 +1,11 @@ import { z } from 'zod'; import { Json, LooseRecord } from '../../../util/schema-utils'; +export const PnpmCatalogsSchema = z.object({ + catalog: z.optional(z.record(z.string())), + catalogs: z.optional(z.record(z.record(z.string()))), +}); + export const PackageManagerSchema = z .string() .transform((val) => val.split('@')) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index eba2094c3f711b..3b4d4d92f5a970 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -3,7 +3,7 @@ import type { Document } from 'yaml'; import { CST, isCollection, isPair, isScalar, parseDocument } from 'yaml'; import { logger } from '../../../../../logger'; import type { UpdateDependencyConfig } from '../../../types'; -import { pnpmCatalogsSchema } from '../../extract/pnpm'; +import { PnpmCatalogsSchema } from '../../schema'; import { getNewGitValue, getNewNpmAliasValue } from './common'; export function updatePnpmCatalogDependency({ @@ -41,7 +41,7 @@ export function updatePnpmCatalogDependency({ // values. Thus, we use both an annotated AST and a JS representation; the // former for manipulation, and the latter for querying/validation. document = parseDocument(fileContent, { keepSourceTokens: true }); - parsedContents = pnpmCatalogsSchema.parse(document.toJS()); + parsedContents = PnpmCatalogsSchema.parse(document.toJS()); } catch (err) { logger.debug({ err }, 'Could not parse pnpm-workspace YAML file.'); return null; From 190458fe1f904dd47edf83fb185b02d487d19d92 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 20:19:17 +0200 Subject: [PATCH 24/46] types: drop Partial from PnpmWorkspaceFile --- lib/modules/manager/npm/extract/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/modules/manager/npm/extract/types.ts b/lib/modules/manager/npm/extract/types.ts index 0abffc5d5744a2..8638dd0cd51a7a 100644 --- a/lib/modules/manager/npm/extract/types.ts +++ b/lib/modules/manager/npm/extract/types.ts @@ -36,8 +36,8 @@ export interface LockFile { export interface PnpmWorkspaceFile { packages: string[]; - catalog?: Partial>; - catalogs?: Partial>>>; + catalog?: Record; + catalogs?: Record>; } /** From 77cf2fec851323b06d1c336607f4de4362d0f19a Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 20:20:10 +0200 Subject: [PATCH 25/46] feat: duck-type pnpm workspace files via schema, to allow renaming --- lib/modules/manager/npm/extract/index.ts | 8 ++++---- lib/modules/manager/npm/extract/pnpm.ts | 9 +++++++++ lib/modules/manager/npm/schema.ts | 6 ++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index 6faaad19ea8e25..18d55fce00b56c 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -17,7 +17,7 @@ import type { import type { NpmLockFiles, NpmManagerData } from '../types'; import { getExtractedConstraints } from './common/dependency'; import { extractPackageJson } from './common/package-file'; -import { extractPnpmWorkspaceFile } from './pnpm'; +import { extractPnpmWorkspaceFile, isPnpmWorkspaceYaml } from './pnpm'; import { postExtract } from './post'; import type { NpmPackage } from './types'; import { isZeroInstall } from './yarn'; @@ -230,9 +230,9 @@ export async function extractAllPackageFiles( const content = await readLocalFile(packageFile, 'utf8'); // istanbul ignore else if (content) { - // TODO(fpapado): for PR dicussion, consider this vs. a post hook, where - // we look for pnpm-workspace.yaml in the siblings - if (packageFile === 'pnpm-workspace.yaml') { + // We duck-type the content here, to allow users to rename the + // pnpm-workspace.yaml file. + if (isPnpmWorkspaceYaml(content)) { const deps = extractPnpmWorkspaceFile(content, packageFile); if (deps) { npmFiles.push({ diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 90079901d0a02c..ea3a16c8981881 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -229,6 +229,15 @@ function getLockedDependencyVersions( return res; } +export function isPnpmWorkspaceYaml(content: string): boolean { + try { + parseSingleYaml(content, { customSchema: PnpmCatalogsSchema }); + return true; + } catch { + return false; + } +} + export function extractPnpmWorkspaceFile( content: string, packageFile: string, diff --git a/lib/modules/manager/npm/schema.ts b/lib/modules/manager/npm/schema.ts index 8e264992dde75b..7efbf9a0f919fb 100644 --- a/lib/modules/manager/npm/schema.ts +++ b/lib/modules/manager/npm/schema.ts @@ -6,6 +6,12 @@ export const PnpmCatalogsSchema = z.object({ catalogs: z.optional(z.record(z.record(z.string()))), }); +export const PnpmWorkspaceFileSchema = z + .object({ + packages: z.array(z.string()), + }) + .and(PnpmCatalogsSchema); + export const PackageManagerSchema = z .string() .transform((val) => val.split('@')) From 283ada53e601596c40ae3653a537d68059570b97 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 20:32:29 +0200 Subject: [PATCH 26/46] feat: use a depName of pnpm.catalog., to allow pattern matching --- lib/modules/manager/npm/extract/pnpm.spec.ts | 8 ++-- lib/modules/manager/npm/extract/pnpm.ts | 33 +++++++--------- .../manager/npm/update/dependency/index.ts | 2 +- .../npm/update/dependency/pnpm.spec.ts | 38 +++++++++---------- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 41b245e8a0af61..3520bacd2b48c1 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -323,8 +323,8 @@ describe('modules/manager/npm/extract/pnpm', () => { currentValue: '18.3.0', datasource: 'npm', depName: 'react', - depType: 'pnpm.catalog', - prettyDepType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', + prettyDepType: 'pnpm.catalog.default', managerData: { catalogName: 'default', }, @@ -333,8 +333,8 @@ describe('modules/manager/npm/extract/pnpm', () => { currentValue: '17.0.2', datasource: 'npm', depName: 'react', - depType: 'pnpm.catalog', - prettyDepType: 'pnpm.catalog', + depType: 'pnpm.catalog.react17', + prettyDepType: 'pnpm.catalog.react17', managerData: { catalogName: 'react17', }, diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index ea3a16c8981881..0ca24f40f1ea4d 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -264,32 +264,28 @@ export function extractPnpmWorkspaceFile( }; } +/** + * In order to facilitate matching on specific catalogs, we structure the + * depType as `pnpm.catalog.default`, `pnpm.catalog.react17`, and so on. + */ +function getCatalogDepType(name: string): string { + const CATALOG_DEPENDENCY = 'pnpm.catalog'; + return `${CATALOG_DEPENDENCY}.${name}`; +} + function extractPnpmCatalogDeps( catalogs: PnpmCatalog[], ): PackageFileContent | null { - const CATALOG_DEPENDENCY = 'pnpm.catalog'; - const deps: PackageDependency[] = []; for (const catalog of catalogs) { for (const [key, val] of Object.entries(catalog.dependencies)) { - const depName = parseDepName(CATALOG_DEPENDENCY, key); + const depType = getCatalogDepType(catalog.name); + const depName = parseDepName(depType, key); let dep: PackageDependency = { - depType: CATALOG_DEPENDENCY, - // TODO(fpapado): for PR discussion, consider how users might be able to - // match on specific catalogs for their config. - // - // For example, we could change depType to `pnpm.catalog.${string}`, so - // that users can match use `{matchDepTypes: ["pnpm.catalog.default"]}`, - // `{matchDepTypes: ["pnpm.catalog.react17"]}` and so on. - // - // Another option would be to mess with depName/packageName. - // - // Is there precedence for something similar? + depType, depName, managerData: { - // We assign the name of the catalog, in order to know which fields to - // update later on. catalogName: catalog.name, }, }; @@ -300,10 +296,9 @@ function extractPnpmCatalogDeps( // TODO: fix type #22198 dep = { ...dep, - ...extractDependency(CATALOG_DEPENDENCY, depName, val!), - prettyDepType: CATALOG_DEPENDENCY, + ...extractDependency(depType, depName, val!), + prettyDepType: depType, }; - dep.prettyDepType = CATALOG_DEPENDENCY; deps.push(dep); } } diff --git a/lib/modules/manager/npm/update/dependency/index.ts b/lib/modules/manager/npm/update/dependency/index.ts index 71efb12d7b5852..0d4bb218f0205e 100644 --- a/lib/modules/manager/npm/update/dependency/index.ts +++ b/lib/modules/manager/npm/update/dependency/index.ts @@ -117,7 +117,7 @@ export function updateDependency({ fileContent, upgrade, }: UpdateDependencyConfig): string | null { - if (upgrade.depType === 'pnpm.catalog') { + if (upgrade.depType?.startsWith('pnpm.catalog')) { return updatePnpmCatalogDependency({ fileContent, upgrade }); } diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 0a470b83a0cb37..1db00977cb1cde 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -4,7 +4,7 @@ import * as npmUpdater from '../..'; describe('modules/manager/npm/update/dependency/pnpm', () => { it('handles implicit default catalog dependency', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -33,7 +33,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('handles explicit default catalog dependency', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -64,7 +64,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('handles explicit named catalog dependency', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.react17', depName: 'react', newValue: '19.0.0', managerData: { @@ -102,7 +102,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('replaces package', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'config', newName: 'abc', newValue: '2.0.0', @@ -132,7 +132,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('replaces a github dependency value', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'gulp', currentValue: 'v4.0.0-alpha.2', currentRawValue: 'gulpjs/gulp#v4.0.0-alpha.2', @@ -163,7 +163,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('replaces a npm package alias', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'hapi', npmPackageAlias: true, packageName: '@hapi/hapi', @@ -195,7 +195,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('replaces a github short hash', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'gulp', currentDigest: 'abcdef7', currentRawValue: 'gulpjs/gulp#abcdef7', @@ -226,7 +226,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('replaces a github fully specified version', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'n', currentValue: 'v1.0.0', currentRawValue: 'git+https://github.com/owner/n#v1.0.0', @@ -257,7 +257,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('returns null if the dependency is not present in the target catalog', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react-not', newValue: '19.0.0', managerData: { @@ -280,7 +280,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('returns null if catalogs are missing', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -300,7 +300,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('returns null if empty file', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -316,7 +316,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves literal whitespace', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -345,7 +345,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves single quote style', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -374,7 +374,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves double quote style', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -406,7 +406,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { // dependencies in catalogs. // @see https://github.com/pnpm/pnpm/issues/8245#issuecomment-2371335323 const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -437,7 +437,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves whitespace with anchors', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -466,7 +466,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves quotation style with anchors', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -495,7 +495,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('preserves formatting in flow style syntax', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { @@ -526,7 +526,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { it('does not replace aliases', () => { const upgrade = { - depType: 'pnpm.catalog', + depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', managerData: { From 6c4ee608af458e6bc2b4416f471ea2ad262c6a9d Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 12 Jan 2025 20:36:19 +0200 Subject: [PATCH 27/46] docs: add mention of `pnpm.catalog` depName in npm readme.md --- lib/modules/manager/npm/readme.md | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/modules/manager/npm/readme.md b/lib/modules/manager/npm/readme.md index daf5ec9582bf7d..09c5110d315474 100644 --- a/lib/modules/manager/npm/readme.md +++ b/lib/modules/manager/npm/readme.md @@ -10,6 +10,7 @@ The following `depTypes` are currently supported by the npm manager : - `overrides` - `resolutions` - `pnpm.overrides` +- `pnpm.catalog.`, such as `pnpm.catalog.default` and `pnpm.catalog.myCatalog`. [Matches any default and named pnpm catalogs](https://pnpm.io/catalogs#defining-catalogs). ### Yarn From bfdae649cf816f42184458217ff55f35499069bd Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Mon, 13 Jan 2025 20:28:41 +0200 Subject: [PATCH 28/46] fix: ensure that pnpm-workspace.yaml duck-typing uses full schema --- lib/modules/manager/npm/extract/pnpm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 0ca24f40f1ea4d..c011484e4bc7e6 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -231,7 +231,7 @@ function getLockedDependencyVersions( export function isPnpmWorkspaceYaml(content: string): boolean { try { - parseSingleYaml(content, { customSchema: PnpmCatalogsSchema }); + parseSingleYaml(content, { customSchema: PnpmWorkspaceFileSchema }); return true; } catch { return false; From 993af1c44b18aad13a65f5eaeb0f613b3f5868a4 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 11:13:20 +0200 Subject: [PATCH 29/46] fix: ensure that pnpm-workspace.yaml updates trigger lockfile updates It turns out that including managerData in one of the extraction deps, would cause the top-level (packageFile) managerData to be overwritten for the corresponding upgrade. In practice, this meant that {managerData: {pnpmShrinkWrap: ''}} was not found, and no lockfile update was triggered. --- lib/modules/manager/npm/extract/index.ts | 10 ++-- lib/modules/manager/npm/extract/pnpm.ts | 51 ++++++++++--------- .../npm/extract/post/locked-versions.ts | 1 + .../manager/npm/update/dependency/pnpm.ts | 2 +- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index 18d55fce00b56c..b0a8f3ac6238c4 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -230,10 +230,13 @@ export async function extractAllPackageFiles( const content = await readLocalFile(packageFile, 'utf8'); // istanbul ignore else if (content) { - // We duck-type the content here, to allow users to rename the - // pnpm-workspace.yaml file. + // pnpm workspace files are their own package file, defined via fileMatch. + // We duck-type the content here, to allow users to rename the file itself. if (isPnpmWorkspaceYaml(content)) { - const deps = extractPnpmWorkspaceFile(content, packageFile); + logger.trace( + `${packageFile} will be extracted as a pnpm workspace YAML file`, + ); + const deps = await extractPnpmWorkspaceFile(content, packageFile); if (deps) { npmFiles.push({ ...deps, @@ -241,6 +244,7 @@ export async function extractAllPackageFiles( }); } } else { + logger.trace(`${packageFile} will be extracted as a package.json file`); const deps = await extractPackageFile(content, packageFile, config); if (deps) { npmFiles.push({ diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index c011484e4bc7e6..31634ad414f3b2 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -16,7 +16,7 @@ import type { PackageFileContent, } from '../../types'; import type { PnpmDependencySchema, PnpmLockFile } from '../post-update/types'; -import { PnpmCatalogsSchema } from '../schema'; +import { PnpmCatalogsSchema, PnpmWorkspaceFileSchema } from '../schema'; import type { NpmManagerData } from '../types'; import { extractDependency, parseDepName } from './common/dependency'; import type { LockFile, PnpmCatalog, PnpmWorkspaceFile } from './types'; @@ -185,6 +185,10 @@ function getLockedVersions( Record> > = {}; + // TODO(fpapado): edit this to include catalogs, in two ways: resolving + // `pnpm.catalog.` depTypes, and resolving `catalog:` dependencies to + // see whether they are locked. + // monorepo if (is.nonEmptyObject(lockParsed.importers)) { for (const [importer, imports] of Object.entries(lockParsed.importers)) { @@ -238,10 +242,10 @@ export function isPnpmWorkspaceYaml(content: string): boolean { } } -export function extractPnpmWorkspaceFile( +export async function extractPnpmWorkspaceFile( content: string, packageFile: string, -): PackageFile | null { +): Promise | null> { logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); const pnpmCatalogs: PnpmCatalog[] = []; @@ -252,15 +256,27 @@ export function extractPnpmWorkspaceFile( return null; } - const extracted = extractPnpmCatalogDeps(pnpmCatalogs); + const deps = extractPnpmCatalogDeps(pnpmCatalogs); - if (!extracted) { + if (!deps) { return null; } + let pnpmShrinkwrap; + const filePath = getSiblingFileName(packageFile, 'pnpm-lock.yaml'); + + if (await readLocalFile(filePath, 'utf8')) { + pnpmShrinkwrap = filePath; + } else { + pnpmShrinkwrap = undefined; + } + return { - ...extracted, - packageFile, + deps, + // TODO(fpapado): Fill out more properties, similar to the rest of the npm manager + managerData: { + pnpmShrinkwrap, + }, }; } @@ -275,27 +291,16 @@ function getCatalogDepType(name: string): string { function extractPnpmCatalogDeps( catalogs: PnpmCatalog[], -): PackageFileContent | null { - const deps: PackageDependency[] = []; +): PackageDependency[] | null { + const deps: PackageDependency[] = []; for (const catalog of catalogs) { for (const [key, val] of Object.entries(catalog.dependencies)) { const depType = getCatalogDepType(catalog.name); const depName = parseDepName(depType, key); - let dep: PackageDependency = { + const dep: PackageDependency = { depType, depName, - managerData: { - catalogName: catalog.name, - }, - }; - if (depName !== key) { - dep.managerData!.key = key; - } - - // TODO: fix type #22198 - dep = { - ...dep, ...extractDependency(depType, depName, val!), prettyDepType: depType, }; @@ -303,9 +308,7 @@ function extractPnpmCatalogDeps( } } - return { - deps, - }; + return deps; } function parsePnpmCatalogs(content: string): PnpmCatalog[] { diff --git a/lib/modules/manager/npm/extract/post/locked-versions.ts b/lib/modules/manager/npm/extract/post/locked-versions.ts index 0970c3faeca509..d5f8720ba7fff7 100644 --- a/lib/modules/manager/npm/extract/post/locked-versions.ts +++ b/lib/modules/manager/npm/extract/post/locked-versions.ts @@ -121,6 +121,7 @@ export async function getLockedVersions( for (const dep of packageFile.deps) { const { depName, depType } = dep; + // TODO(fpapado): Consider looking up `pnpm.catalogs.` types here // TODO: types (#22198) const lockedVersion = semver.valid( lockFileCache[pnpmShrinkwrap].lockedVersionsWithPath?.[relativeDir]?.[ diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 3b4d4d92f5a970..679982adb38f3b 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -12,7 +12,7 @@ export function updatePnpmCatalogDependency({ }: UpdateDependencyConfig): string | null { const { depType, managerData, depName } = upgrade; - const catalogName = managerData?.catalogName; + const catalogName = depType?.split('.').at(-1); if (!is.string(catalogName)) { logger.error( From 9b7807dbf51e33fa3acd592ca40701ef773303a7 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 11:47:48 +0200 Subject: [PATCH 30/46] test: fix pnpm tests, by awaiting and by changing the object shape --- lib/modules/manager/npm/extract/pnpm.spec.ts | 21 +++---- .../npm/update/dependency/pnpm.spec.ts | 57 ------------------- 2 files changed, 8 insertions(+), 70 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 3520bacd2b48c1..f80f55ea85e19f 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -288,13 +288,15 @@ describe('modules/manager/npm/extract/pnpm', () => { }); describe('.extractPnpmWorkspaceFile()', () => { - it('ignores invalid pnpm-workspace.yaml file', () => { - expect(extractPnpmWorkspaceFile('', 'pnpm-workspace.yaml')).toBeNull(); + it('ignores invalid pnpm-workspace.yaml file', async () => { + expect( + await extractPnpmWorkspaceFile('', 'pnpm-workspace.yaml'), + ).toBeNull(); }); - it('handles empty catalog entries', () => { + it('handles empty catalog entries', async () => { expect( - extractPnpmWorkspaceFile( + await extractPnpmWorkspaceFile( codeBlock` catalog: catalogs: @@ -304,9 +306,9 @@ describe('modules/manager/npm/extract/pnpm', () => { ).toBeNull(); }); - it('parses valid pnpm-workspace.yaml file', () => { + it('parses valid pnpm-workspace.yaml file', async () => { expect( - extractPnpmWorkspaceFile( + await extractPnpmWorkspaceFile( codeBlock` catalog: react: 18.3.0 @@ -325,9 +327,6 @@ describe('modules/manager/npm/extract/pnpm', () => { depName: 'react', depType: 'pnpm.catalog.default', prettyDepType: 'pnpm.catalog.default', - managerData: { - catalogName: 'default', - }, }, { currentValue: '17.0.2', @@ -335,12 +334,8 @@ describe('modules/manager/npm/extract/pnpm', () => { depName: 'react', depType: 'pnpm.catalog.react17', prettyDepType: 'pnpm.catalog.react17', - managerData: { - catalogName: 'react17', - }, }, ], - packageFile: 'pnpm-workspace.yaml', }); }); }); diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 1db00977cb1cde..6cae040b625c55 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -7,9 +7,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -36,9 +33,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -67,9 +61,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.react17', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'react17', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -106,9 +97,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depName: 'config', newName: 'abc', newValue: '2.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -137,9 +125,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { currentValue: 'v4.0.0-alpha.2', currentRawValue: 'gulpjs/gulp#v4.0.0-alpha.2', newValue: 'v4.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -169,9 +154,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { packageName: '@hapi/hapi', currentValue: '18.3.0', newValue: '18.3.1', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -200,9 +182,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { currentDigest: 'abcdef7', currentRawValue: 'gulpjs/gulp#abcdef7', newDigest: '0000000000111111111122222222223333333333', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -231,9 +210,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { currentValue: 'v1.0.0', currentRawValue: 'git+https://github.com/owner/n#v1.0.0', newValue: 'v1.1.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -260,9 +236,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react-not', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -283,9 +256,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -303,9 +273,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const testContent = npmUpdater.updateDependency({ fileContent: null as never, @@ -319,9 +286,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -348,9 +312,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -377,9 +338,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -409,9 +367,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -440,9 +395,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -469,9 +421,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -498,9 +447,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; const pnpmWorkspaceYaml = codeBlock` packages: @@ -529,9 +475,6 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { depType: 'pnpm.catalog.default', depName: 'react', newValue: '19.0.0', - managerData: { - catalogName: 'default', - }, }; // In the general case, we do not know whether we should replace the anchor // that an alias is resolved from. We leave this up to the user, e.g. via a From 8b596f313ffd41760d71e023f3d51a3e52d63d8b Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 12:05:39 +0200 Subject: [PATCH 31/46] test: fix tests that do assertions on the shape of npm manager config Adding pnpm-workspace.yaml to the default matchFiles means that a few of the assertions were failing. --- lib/config/index.spec.ts | 2 +- .../extract/extract-fingerprint-config.spec.ts | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/config/index.spec.ts b/lib/config/index.spec.ts index 13fd4ac4fc995b..9cc5c378ba919c 100644 --- a/lib/config/index.spec.ts +++ b/lib/config/index.spec.ts @@ -125,7 +125,7 @@ describe('config/index', () => { const parentConfig = { ...defaultConfig }; const config = getManagerConfig(parentConfig, 'npm'); expect(config).toContainEntries([ - ['fileMatch', ['(^|/)package\\.json$']], + ['fileMatch', ['(^|/)package\\.json$', '(^|/)pnpm-workspace\\.yaml$']], ]); expect(getManagerConfig(parentConfig, 'html')).toContainEntries([ ['fileMatch', ['\\.html?$']], diff --git a/lib/workers/repository/extract/extract-fingerprint-config.spec.ts b/lib/workers/repository/extract/extract-fingerprint-config.spec.ts index 3f8fe77f905966..eca0d3a980771c 100644 --- a/lib/workers/repository/extract/extract-fingerprint-config.spec.ts +++ b/lib/workers/repository/extract/extract-fingerprint-config.spec.ts @@ -39,7 +39,11 @@ describe('workers/repository/extract/extract-fingerprint-config', () => { ).toEqual({ enabled: true, fileList: [], - fileMatch: ['(^|/)package\\.json$', 'hero.json'], + fileMatch: [ + '(^|/)package\\.json$', + '(^|/)pnpm-workspace\\.yaml$', + 'hero.json', + ], ignorePaths: ['ignore-path-2'], includePaths: ['include-path-2'], manager: 'npm', @@ -85,7 +89,11 @@ describe('workers/repository/extract/extract-fingerprint-config', () => { ).toEqual({ enabled: true, fileList: [], - fileMatch: ['(^|/)package\\.json$', 'hero.json'], + fileMatch: [ + '(^|/)package\\.json$', + '(^|/)pnpm-workspace\\.yaml$', + 'hero.json', + ], ignorePaths: ['**/node_modules/**', '**/bower_components/**'], includePaths: [], manager: 'npm', From 1b349e9d27ce3202be646dfa05e10facb4bdfc9c Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 12:43:41 +0200 Subject: [PATCH 32/46] test: address coverage in lib/modules/manager/npm/update --- .../npm/update/dependency/pnpm.spec.ts | 45 ++++++++++++++++++- .../manager/npm/update/dependency/pnpm.ts | 4 +- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index 6cae040b625c55..f0d4640ebd16c8 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -91,6 +91,26 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { `); }); + it('does nothing if the new and old values match', () => { + const upgrade = { + depType: 'pnpm.catalog.default', + depName: 'react', + newValue: '19.0.0', + }; + const pnpmWorkspaceYaml = codeBlock` + packages: + - pkg-a + + catalog: + react: 19.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(pnpmWorkspaceYaml); + }); + it('replaces package', () => { const upgrade = { depType: 'pnpm.catalog.default', @@ -470,7 +490,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { `); }); - it('does not replace aliases', () => { + it('does not replace aliases in the value position', () => { const upgrade = { depType: 'pnpm.catalog.default', depName: 'react', @@ -496,4 +516,27 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { }); expect(testContent).toBeNull(); }); + + it('does not replace aliases in the key position', () => { + const upgrade = { + depType: 'pnpm.catalog.default', + depName: 'react', + newName: 'react-x', + }; + const pnpmWorkspaceYaml = codeBlock` + __vars: + &r react: "" + + packages: + - pkg-a + + catalog: + *r: 18.0.0 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toBeNull(); + }); }); diff --git a/lib/modules/manager/npm/update/dependency/pnpm.ts b/lib/modules/manager/npm/update/dependency/pnpm.ts index 679982adb38f3b..56ecbb03a45df9 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.ts @@ -14,6 +14,7 @@ export function updatePnpmCatalogDependency({ const catalogName = depType?.split('.').at(-1); + // istanbul ignore if if (!is.string(catalogName)) { logger.error( 'No catalogName was found; this is likely an extraction error.', @@ -81,8 +82,8 @@ export function updatePnpmCatalogDependency({ return null; } + // istanbul ignore if: this should not happen in practice, but we must satisfy th etypes if (!modifiedDocument.contents?.srcToken) { - // This should not happen in practice, but we leave it to satisfy the types. return null; } @@ -117,6 +118,7 @@ function changeDependencyIn( } if (newName) { + // istanbul ignore if: the try..catch block above already throws if a key is an alias if (!CST.isScalar(relevantNode.srcToken?.key)) { return null; } From 5b90887897a719e6c3d2533f4ec9e8098bd386aa Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 12:45:24 +0200 Subject: [PATCH 33/46] chore: remove done TODO --- lib/modules/manager/npm/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/modules/manager/npm/index.ts b/lib/modules/manager/npm/index.ts index e8550072f49c72..1c120457c43243 100644 --- a/lib/modules/manager/npm/index.ts +++ b/lib/modules/manager/npm/index.ts @@ -20,7 +20,6 @@ export const url = 'https://docs.npmjs.com'; export const categories: Category[] = ['js']; export const defaultConfig = { - // TODO(fpapado): for PR dicussion, consider this vs. a post hook fileMatch: ['(^|/)package\\.json$', '(^|/)pnpm-workspace\\.yaml$'], digest: { prBodyDefinitions: { From 77678e19ebe1d8abb686c88c44d692bed8ea14d7 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 13:46:08 +0200 Subject: [PATCH 34/46] test: address coverage in lib/modules/manager/npm/extract/ --- lib/modules/manager/npm/extract/index.spec.ts | 30 +++++++++++++++++++ lib/modules/manager/npm/extract/pnpm.ts | 8 +---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.spec.ts b/lib/modules/manager/npm/extract/index.spec.ts index ec54ac88b183d7..a73936ce273b9a 100644 --- a/lib/modules/manager/npm/extract/index.spec.ts +++ b/lib/modules/manager/npm/extract/index.spec.ts @@ -1159,6 +1159,36 @@ describe('modules/manager/npm/extract/index', () => { }, ]); }); + + it('extracts pnpm workspace yaml files', async () => { + fs.readLocalFile.mockResolvedValueOnce(codeBlock` + packages: + - pkg-a + + catalog: + is-positive: 1.0.0 + `); + const res = await extractAllPackageFiles(defaultExtractConfig, [ + 'pnpm-workspace.yaml', + ]); + expect(res).toEqual([ + { + deps: [ + { + currentValue: '1.0.0', + datasource: 'npm', + depName: 'is-positive', + depType: 'pnpm.catalog.default', + prettyDepType: 'pnpm.catalog.default', + }, + ], + managerData: { + pnpmShrinkwrap: undefined, + }, + packageFile: 'pnpm-workspace.yaml', + }, + ]); + }); }); describe('.postExtract()', () => { diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 31634ad414f3b2..f5e882fd6e2ef5 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -258,17 +258,11 @@ export async function extractPnpmWorkspaceFile( const deps = extractPnpmCatalogDeps(pnpmCatalogs); - if (!deps) { - return null; - } - let pnpmShrinkwrap; const filePath = getSiblingFileName(packageFile, 'pnpm-lock.yaml'); if (await readLocalFile(filePath, 'utf8')) { pnpmShrinkwrap = filePath; - } else { - pnpmShrinkwrap = undefined; } return { @@ -291,7 +285,7 @@ function getCatalogDepType(name: string): string { function extractPnpmCatalogDeps( catalogs: PnpmCatalog[], -): PackageDependency[] | null { +): PackageDependency[] { const deps: PackageDependency[] = []; for (const catalog of catalogs) { From 668c5110cea3bc99c1452a8d6f3f312940cd3a74 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 18:15:07 +0200 Subject: [PATCH 35/46] feat: extract locked versions from catalogs --- .../lockfile-parsing/pnpm-lock-v9.yaml | 44 ++++++++++++++ lib/modules/manager/npm/extract/pnpm.spec.ts | 10 ++++ lib/modules/manager/npm/extract/pnpm.ts | 26 ++++++-- .../npm/extract/post/locked-versions.spec.ts | 60 +++++++++++++++++++ .../npm/extract/post/locked-versions.ts | 37 +++++++++--- lib/modules/manager/npm/extract/types.ts | 1 + lib/modules/manager/npm/post-update/types.ts | 1 + 7 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml diff --git a/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml b/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml new file mode 100644 index 00000000000000..6ab7ed0dc146a2 --- /dev/null +++ b/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml @@ -0,0 +1,44 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +catalogs: + default: + react: + specifier: ^18 + version: 18.3.1 + +importers: + + .: + dependencies: + react: + specifier: 'catalog:' + version: 18.3.1 + +packages: + + js-tokens@4.0.0: + resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} + + loose-envify@1.4.0: + resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} + hasBin: true + + react@18.3.1: + resolution: {integrity: sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==} + engines: {node: '>=0.10.0'} + +snapshots: + + js-tokens@4.0.0: {} + + loose-envify@1.4.0: + dependencies: + js-tokens: 4.0.0 + + react@18.3.1: + dependencies: + loose-envify: 1.4.0 diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index f80f55ea85e19f..d68b313c14f4d6 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -280,6 +280,16 @@ describe('modules/manager/npm/extract/pnpm', () => { expect(Object.keys(res.lockedVersionsWithPath!)).toHaveLength(1); }); + it('extracts version from catalogs', async () => { + const plocktest1Lock = Fixtures.get( + 'lockfile-parsing/pnpm-lock-v9.yaml', + '..', + ); + jest.spyOn(fs, 'readLocalFile').mockResolvedValueOnce(plocktest1Lock); + const res = await getPnpmLock('package.json'); + expect(Object.keys(res.lockedVersionsWithCatalog!)).toHaveLength(1); + }); + it('returns empty if no deps', async () => { fs.readLocalFile.mockResolvedValueOnce('{}'); const res = await getPnpmLock('package.json'); diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index f5e882fd6e2ef5..a6098c11a026ce 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -166,9 +166,11 @@ export async function getPnpmLock(filePath: string): Promise { : parseFloat(lockParsed.lockfileVersion); const lockedVersions = getLockedVersions(lockParsed); + const lockedCatalogVersions = getLockedCatalogVersions(lockParsed); return { lockedVersionsWithPath: lockedVersions, + lockedVersionsWithCatalog: lockedCatalogVersions, lockfileVersion, }; } catch (err) { @@ -177,6 +179,26 @@ export async function getPnpmLock(filePath: string): Promise { } } +function getLockedCatalogVersions( + lockParsed: PnpmLockFile, +): Record> { + const lockedVersions: Record> = {}; + + if (is.nonEmptyObject(lockParsed.catalogs)) { + for (const [catalog, dependencies] of Object.entries(lockParsed.catalogs)) { + const versions: Record = {}; + + for (const [dep, versionCarrier] of Object.entries(dependencies)) { + versions[dep] = versionCarrier.version; + } + + lockedVersions[catalog] = versions; + } + } + + return lockedVersions; +} + function getLockedVersions( lockParsed: PnpmLockFile, ): Record>> { @@ -185,10 +207,6 @@ function getLockedVersions( Record> > = {}; - // TODO(fpapado): edit this to include catalogs, in two ways: resolving - // `pnpm.catalog.` depTypes, and resolving `catalog:` dependencies to - // see whether they are locked. - // monorepo if (is.nonEmptyObject(lockParsed.importers)) { for (const [importer, imports] of Object.entries(lockParsed.importers)) { diff --git a/lib/modules/manager/npm/extract/post/locked-versions.spec.ts b/lib/modules/manager/npm/extract/post/locked-versions.spec.ts index e141766d63f973..fffffa6d567eda 100644 --- a/lib/modules/manager/npm/extract/post/locked-versions.spec.ts +++ b/lib/modules/manager/npm/extract/post/locked-versions.spec.ts @@ -586,6 +586,66 @@ describe('modules/manager/npm/extract/post/locked-versions', () => { ]); }); + it('uses pnpm-lock for pnpm.catalog depType', async () => { + pnpm.getPnpmLock.mockResolvedValue({ + lockedVersionsWithCatalog: { + default: { + a: '1.0.0', + }, + named: { + b: '2.0.0', + }, + }, + lockfileVersion: 9.0, + }); + const packageFiles = [ + { + managerData: { + pnpmShrinkwrap: 'pnpm-lock.yaml', + }, + extractedConstraints: { + pnpm: '9.15.3', + }, + deps: [ + { + depName: 'a', + depType: 'pnpm.catalog.default', + currentValue: '1.0.0', + }, + { + depName: 'b', + depType: 'pnpm.catalog.named', + currentValue: '2.0.0', + }, + ], + packageFile: 'pnpm-workspace.yaml', + }, + ]; + await getLockedVersions(packageFiles); + expect(packageFiles).toEqual([ + { + extractedConstraints: { pnpm: '9.15.3' }, + deps: [ + { + currentValue: '1.0.0', + depName: 'a', + lockedVersion: '1.0.0', + depType: 'pnpm.catalog.default', + }, + { + currentValue: '2.0.0', + depName: 'b', + lockedVersion: '2.0.0', + depType: 'pnpm.catalog.named', + }, + ], + lockFiles: ['pnpm-lock.yaml'], + managerData: { pnpmShrinkwrap: 'pnpm-lock.yaml' }, + packageFile: 'pnpm-workspace.yaml', + }, + ]); + }); + it('uses pnpm-lock in subfolder', async () => { pnpm.getPnpmLock.mockResolvedValue({ lockedVersionsWithPath: { diff --git a/lib/modules/manager/npm/extract/post/locked-versions.ts b/lib/modules/manager/npm/extract/post/locked-versions.ts index d5f8720ba7fff7..c86a4e67424542 100644 --- a/lib/modules/manager/npm/extract/post/locked-versions.ts +++ b/lib/modules/manager/npm/extract/post/locked-versions.ts @@ -8,6 +8,9 @@ import { getNpmLock } from '../npm'; import { getPnpmLock } from '../pnpm'; import type { LockFile } from '../types'; import { getYarnLock, getYarnVersionFromLock } from '../yarn'; + +const pnpmCatalogDepTypeRe = /pnpm\.catalog\.(?.*)/; + export async function getLockedVersions( packageFiles: PackageFile[], ): Promise { @@ -121,15 +124,31 @@ export async function getLockedVersions( for (const dep of packageFile.deps) { const { depName, depType } = dep; - // TODO(fpapado): Consider looking up `pnpm.catalogs.` types here - // TODO: types (#22198) - const lockedVersion = semver.valid( - lockFileCache[pnpmShrinkwrap].lockedVersionsWithPath?.[relativeDir]?.[ - depType! - ]?.[depName!], - ); - if (is.string(lockedVersion)) { - dep.lockedVersion = lockedVersion; + + const catalogName = pnpmCatalogDepTypeRe.exec(depType!)?.groups + ?.version; + + if (catalogName) { + const lockedVersion = semver.valid( + lockFileCache[pnpmShrinkwrap].lockedVersionsWithCatalog?.[ + catalogName + ]?.[depName!], + ); + + if (is.string(lockedVersion)) { + dep.lockedVersion = lockedVersion; + } + } else { + // TODO: types (#22198) + const lockedVersion = semver.valid( + lockFileCache[pnpmShrinkwrap].lockedVersionsWithPath?.[ + relativeDir + ]?.[depType!]?.[depName!], + ); + + if (is.string(lockedVersion)) { + dep.lockedVersion = lockedVersion; + } } } } diff --git a/lib/modules/manager/npm/extract/types.ts b/lib/modules/manager/npm/extract/types.ts index 8638dd0cd51a7a..46f7fb220fc12a 100644 --- a/lib/modules/manager/npm/extract/types.ts +++ b/lib/modules/manager/npm/extract/types.ts @@ -30,6 +30,7 @@ export interface LockFile { string, Record> >; + lockedVersionsWithCatalog?: Record>; lockfileVersion?: number; // cache version for Yarn isYarn1?: boolean; } diff --git a/lib/modules/manager/npm/post-update/types.ts b/lib/modules/manager/npm/post-update/types.ts index 35efbdd2f790e4..2ffe4d7f41bf0f 100644 --- a/lib/modules/manager/npm/post-update/types.ts +++ b/lib/modules/manager/npm/post-update/types.ts @@ -35,6 +35,7 @@ export type PnpmDependencySchema = Record; export interface PnpmLockFile { lockfileVersion: number | string; + catalogs?: Record>; importers?: Record>; dependencies: PnpmDependencySchema; devDependencies: PnpmDependencySchema; From 6cfd893cf5f37d678462c33da4793aa66c6df1a6 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 18:17:18 +0200 Subject: [PATCH 36/46] chore: remove done TODO --- lib/modules/manager/npm/extract/pnpm.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index a6098c11a026ce..6479e862c12b30 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -285,7 +285,6 @@ export async function extractPnpmWorkspaceFile( return { deps, - // TODO(fpapado): Fill out more properties, similar to the rest of the npm manager managerData: { pnpmShrinkwrap, }, From b886aa6d557b68652315e7612aa849e412a57876 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 18:47:46 +0200 Subject: [PATCH 37/46] test: tidy up coverage --- lib/modules/manager/npm/extract/pnpm.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 6479e862c12b30..2aa4382ddf327a 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -92,8 +92,8 @@ export async function detectPnpmWorkspaces( const packagePathCache = new Map(); for (const p of packageFiles) { - const { packageFile, managerData = {} } = p; - const { pnpmShrinkwrap } = managerData as Partial; + const { packageFile, managerData } = p; + const pnpmShrinkwrap = managerData?.pnpmShrinkwrap; // check if pnpmShrinkwrap-file has already been provided if (pnpmShrinkwrap) { @@ -326,12 +326,11 @@ function parsePnpmCatalogs(content: string): PnpmCatalog[] { const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = parseSingleYaml(content, { customSchema: PnpmCatalogsSchema }); - const result = [ - { - name: 'default', - dependencies: defaultCatalogDeps ?? {}, - }, - ]; + const result = []; + + if (defaultCatalogDeps !== undefined) { + result.push({ name: 'default', dependencies: defaultCatalogDeps }); + } if (!namedCatalogs) { return result; From 155523523ad777dc2884465cbcb750b18c650011 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 21:49:08 +0200 Subject: [PATCH 38/46] fix: ensure that pnpm-workspace.yaml artifacts get written prior to lockfile This ensures that the lockfile update command will see the updated workspace file, and will update correctly. There was a hardcoded package.json check, that was previously precluding that. --- lib/modules/manager/npm/post-update/index.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/modules/manager/npm/post-update/index.ts b/lib/modules/manager/npm/post-update/index.ts index 5b3c139175b566..14be340951e127 100644 --- a/lib/modules/manager/npm/post-update/index.ts +++ b/lib/modules/manager/npm/post-update/index.ts @@ -242,7 +242,12 @@ export async function writeUpdatedPackageFiles( await writeLocalFile(packageFile.path, packageFile.contents!); continue; } - if (!packageFile.path.endsWith('package.json')) { + if ( + !( + packageFile.path.endsWith('package.json') || + packageFile.path.endsWith('pnpm-workspace.yaml') + ) + ) { continue; } logger.debug(`Writing ${packageFile.path}`); @@ -609,6 +614,10 @@ export async function getAdditionalFiles( pnpmShrinkwrap, config.reuseExistingBranch ? config.branchName : config.baseBranch, ); + logger.info( + { lockFile: res.lockFile, existingContent }, + 'FP: lockfile comparison', + ); if (res.lockFile === existingContent) { logger.debug("pnpm-lock.yaml hasn't changed"); } else { From ea535e99631896618ba19cd99249ff82c3d15d42 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 22:12:52 +0200 Subject: [PATCH 39/46] test: add explicit tests for comments in pnpm workspace updates --- .../npm/update/dependency/pnpm.spec.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts index f0d4640ebd16c8..211371239b004d 100644 --- a/lib/modules/manager/npm/update/dependency/pnpm.spec.ts +++ b/lib/modules/manager/npm/update/dependency/pnpm.spec.ts @@ -353,6 +353,36 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { `); }); + it('preserves comments', () => { + const upgrade = { + depType: 'pnpm.catalog.default', + depName: 'react', + newValue: '19.0.0', + }; + const pnpmWorkspaceYaml = codeBlock` + packages: + - pkg-a + + catalog: + react: 18.3.1 # This is a comment + # This is another comment + react-dom: 18.3.1 + `; + const testContent = npmUpdater.updateDependency({ + fileContent: pnpmWorkspaceYaml, + upgrade, + }); + expect(testContent).toEqual(codeBlock` + packages: + - pkg-a + + catalog: + react: 19.0.0 # This is a comment + # This is another comment + react-dom: 18.3.1 + `); + }); + it('preserves double quote style', () => { const upgrade = { depType: 'pnpm.catalog.default', @@ -473,6 +503,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { - pkg-a catalog: { + # This is a comment "react": "18.3.1" } `; @@ -485,6 +516,7 @@ describe('modules/manager/npm/update/dependency/pnpm', () => { - pkg-a catalog: { + # This is a comment "react": "19.0.0" } `); From d6820f75e217d5bc79d11500a1cb4ec564aed347 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 14 Jan 2025 22:27:26 +0200 Subject: [PATCH 40/46] chore: remove errant logging --- lib/modules/manager/npm/post-update/index.ts | 4 ---- lib/modules/manager/npm/update/dependency/common.ts | 1 - 2 files changed, 5 deletions(-) diff --git a/lib/modules/manager/npm/post-update/index.ts b/lib/modules/manager/npm/post-update/index.ts index 14be340951e127..6aafc2ef08f474 100644 --- a/lib/modules/manager/npm/post-update/index.ts +++ b/lib/modules/manager/npm/post-update/index.ts @@ -614,10 +614,6 @@ export async function getAdditionalFiles( pnpmShrinkwrap, config.reuseExistingBranch ? config.branchName : config.baseBranch, ); - logger.info( - { lockFile: res.lockFile, existingContent }, - 'FP: lockfile comparison', - ); if (res.lockFile === existingContent) { logger.debug("pnpm-lock.yaml hasn't changed"); } else { diff --git a/lib/modules/manager/npm/update/dependency/common.ts b/lib/modules/manager/npm/update/dependency/common.ts index 92c9172677805d..7a84d4e21f5783 100644 --- a/lib/modules/manager/npm/update/dependency/common.ts +++ b/lib/modules/manager/npm/update/dependency/common.ts @@ -10,7 +10,6 @@ export function getNewGitValue(upgrade: Upgrade): string | null { return upgrade.currentRawValue.replace( upgrade.currentDigest, // TODO #22198 - upgrade.newDigest!.substring(0, upgrade.currentDigest.length), ); } else { From bfcc4e98b933f399ffec77461c7b4d06844568b2 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 12:42:05 +0200 Subject: [PATCH 41/46] Apply suggestions from code review Co-authored-by: Sebastian Poxhofer --- lib/modules/manager/npm/extract/index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index b0a8f3ac6238c4..00a80fbe2bbc99 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -233,9 +233,7 @@ export async function extractAllPackageFiles( // pnpm workspace files are their own package file, defined via fileMatch. // We duck-type the content here, to allow users to rename the file itself. if (isPnpmWorkspaceYaml(content)) { - logger.trace( - `${packageFile} will be extracted as a pnpm workspace YAML file`, - ); + logger.trace({packageFile}, `Extracting file as a pnpm workspace YAML file`); const deps = await extractPnpmWorkspaceFile(content, packageFile); if (deps) { npmFiles.push({ @@ -244,7 +242,7 @@ export async function extractAllPackageFiles( }); } } else { - logger.trace(`${packageFile} will be extracted as a package.json file`); + logger.trace({ packageFile }, `Extracting as a package.json file`); const deps = await extractPackageFile(content, packageFile, config); if (deps) { npmFiles.push({ From c8062e00680940c4432ee5ef18c6cefd7ee50a14 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 12:46:14 +0200 Subject: [PATCH 42/46] test: inline npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml --- .../lockfile-parsing/pnpm-lock-v9.yaml | 44 ---------------- lib/modules/manager/npm/extract/pnpm.spec.ts | 52 +++++++++++++++++-- 2 files changed, 47 insertions(+), 49 deletions(-) delete mode 100644 lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml diff --git a/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml b/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml deleted file mode 100644 index 6ab7ed0dc146a2..00000000000000 --- a/lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml +++ /dev/null @@ -1,44 +0,0 @@ -lockfileVersion: '9.0' - -settings: - autoInstallPeers: true - excludeLinksFromLockfile: false - -catalogs: - default: - react: - specifier: ^18 - version: 18.3.1 - -importers: - - .: - dependencies: - react: - specifier: 'catalog:' - version: 18.3.1 - -packages: - - js-tokens@4.0.0: - resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} - - loose-envify@1.4.0: - resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} - hasBin: true - - react@18.3.1: - resolution: {integrity: sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==} - engines: {node: '>=0.10.0'} - -snapshots: - - js-tokens@4.0.0: {} - - loose-envify@1.4.0: - dependencies: - js-tokens: 4.0.0 - - react@18.3.1: - dependencies: - loose-envify: 1.4.0 diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index d68b313c14f4d6..93c2fcfcebe623 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -281,11 +281,53 @@ describe('modules/manager/npm/extract/pnpm', () => { }); it('extracts version from catalogs', async () => { - const plocktest1Lock = Fixtures.get( - 'lockfile-parsing/pnpm-lock-v9.yaml', - '..', - ); - jest.spyOn(fs, 'readLocalFile').mockResolvedValueOnce(plocktest1Lock); + const lockfileContent = codeBlock` + lockfileVersion: '9.0' + + settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + + catalogs: + default: + react: + specifier: ^18 + version: 18.3.1 + + importers: + + .: + dependencies: + react: + specifier: 'catalog:' + version: 18.3.1 + + packages: + + js-tokens@4.0.0: + resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} + + loose-envify@1.4.0: + resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} + hasBin: true + + react@18.3.1: + resolution: {integrity: sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==} + engines: {node: '>=0.10.0'} + + snapshots: + + js-tokens@4.0.0: {} + + loose-envify@1.4.0: + dependencies: + js-tokens: 4.0.0 + + react@18.3.1: + dependencies: + loose-envify: 1.4.0 + `; + jest.spyOn(fs, 'readLocalFile').mockResolvedValueOnce(lockfileContent); const res = await getPnpmLock('package.json'); expect(Object.keys(res.lockedVersionsWithCatalog!)).toHaveLength(1); }); From c3e69f218f687941ed47cf7d7d0d2f43db1f4961 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 13:32:21 +0200 Subject: [PATCH 43/46] test: use fs from test utils in npm/extract/pnpm.spec.ts This required a few more changes in the rest of the file, in order to mock fs and use the utils consistently. --- lib/modules/manager/npm/extract/pnpm.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 93c2fcfcebe623..41027e75478f99 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -327,7 +327,7 @@ describe('modules/manager/npm/extract/pnpm', () => { dependencies: loose-envify: 1.4.0 `; - jest.spyOn(fs, 'readLocalFile').mockResolvedValueOnce(lockfileContent); + fs.readLocalFile.mockResolvedValueOnce(lockfileContent); const res = await getPnpmLock('package.json'); expect(Object.keys(res.lockedVersionsWithCatalog!)).toHaveLength(1); }); From c67103675f7eb2a14899449488a725a296f00407 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 19:06:03 +0200 Subject: [PATCH 44/46] perf: avoid double-parsing when detecting workspace file This introduces a tryParsePnpmWorkspaceYaml function, which parses a string as YAML, and validates it via a schema, returning the result, and whether parsing succeeded. This allows the npm manager extraction call-site to only parse the content once, and pass on the parsed contents (if any) for extraction. --- lib/modules/manager/npm/extract/index.ts | 15 ++++++-- lib/modules/manager/npm/extract/pnpm.spec.ts | 33 +++++++---------- lib/modules/manager/npm/extract/pnpm.ts | 39 +++++++++++--------- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/lib/modules/manager/npm/extract/index.ts b/lib/modules/manager/npm/extract/index.ts index 00a80fbe2bbc99..d96f38bac16643 100644 --- a/lib/modules/manager/npm/extract/index.ts +++ b/lib/modules/manager/npm/extract/index.ts @@ -17,7 +17,7 @@ import type { import type { NpmLockFiles, NpmManagerData } from '../types'; import { getExtractedConstraints } from './common/dependency'; import { extractPackageJson } from './common/package-file'; -import { extractPnpmWorkspaceFile, isPnpmWorkspaceYaml } from './pnpm'; +import { extractPnpmWorkspaceFile, tryParsePnpmWorkspaceYaml } from './pnpm'; import { postExtract } from './post'; import type { NpmPackage } from './types'; import { isZeroInstall } from './yarn'; @@ -232,9 +232,16 @@ export async function extractAllPackageFiles( if (content) { // pnpm workspace files are their own package file, defined via fileMatch. // We duck-type the content here, to allow users to rename the file itself. - if (isPnpmWorkspaceYaml(content)) { - logger.trace({packageFile}, `Extracting file as a pnpm workspace YAML file`); - const deps = await extractPnpmWorkspaceFile(content, packageFile); + const parsedPnpmWorkspaceYaml = tryParsePnpmWorkspaceYaml(content); + if (parsedPnpmWorkspaceYaml.success) { + logger.trace( + { packageFile }, + `Extracting file as a pnpm workspace YAML file`, + ); + const deps = await extractPnpmWorkspaceFile( + parsedPnpmWorkspaceYaml.data, + packageFile, + ); if (deps) { npmFiles.push({ ...deps, diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 41027e75478f99..4ec565245c99ce 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -340,35 +340,30 @@ describe('modules/manager/npm/extract/pnpm', () => { }); describe('.extractPnpmWorkspaceFile()', () => { - it('ignores invalid pnpm-workspace.yaml file', async () => { - expect( - await extractPnpmWorkspaceFile('', 'pnpm-workspace.yaml'), - ).toBeNull(); - }); - it('handles empty catalog entries', async () => { expect( await extractPnpmWorkspaceFile( - codeBlock` - catalog: - catalogs: - `, + { catalog: {}, catalogs: {} }, 'pnpm-workspace.yaml', ), - ).toBeNull(); + ).toMatchObject({ + deps: [], + }); }); it('parses valid pnpm-workspace.yaml file', async () => { expect( await extractPnpmWorkspaceFile( - codeBlock` - catalog: - react: 18.3.0 - - catalogs: - react17: - react: 17.0.2 - `, + { + catalog: { + react: '18.3.0', + }, + catalogs: { + react17: { + react: '17.0.2', + }, + }, + }, 'pnpm-workspace.yaml', ), ).toMatchObject({ diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 2aa4382ddf327a..276cf679d6d5c9 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -1,6 +1,7 @@ import is from '@sindresorhus/is'; import { findPackages } from 'find-packages'; import upath from 'upath'; +import type { z } from 'zod'; import { GlobalConfig } from '../../../../config/global'; import { logger } from '../../../../logger'; import { @@ -16,7 +17,8 @@ import type { PackageFileContent, } from '../../types'; import type { PnpmDependencySchema, PnpmLockFile } from '../post-update/types'; -import { PnpmCatalogsSchema, PnpmWorkspaceFileSchema } from '../schema'; +import type { PnpmCatalogsSchema } from '../schema'; +import { PnpmWorkspaceFileSchema } from '../schema'; import type { NpmManagerData } from '../types'; import { extractDependency, parseDepName } from './common/dependency'; import type { LockFile, PnpmCatalog, PnpmWorkspaceFile } from './types'; @@ -251,28 +253,31 @@ function getLockedDependencyVersions( return res; } -export function isPnpmWorkspaceYaml(content: string): boolean { +export function tryParsePnpmWorkspaceYaml(content: string): + | { + success: true; + data: PnpmWorkspaceFile; + } + | { success: false; data?: never } { try { - parseSingleYaml(content, { customSchema: PnpmWorkspaceFileSchema }); - return true; + const data = parseSingleYaml(content, { + customSchema: PnpmWorkspaceFileSchema, + }); + return { success: true, data }; } catch { - return false; + return { success: false }; } } +type PnpmCatalogs = z.TypeOf; + export async function extractPnpmWorkspaceFile( - content: string, + catalogs: PnpmCatalogs, packageFile: string, ): Promise | null> { logger.trace(`pnpm.extractPnpmWorkspaceFile(${packageFile})`); - const pnpmCatalogs: PnpmCatalog[] = []; - try { - pnpmCatalogs.push(...parsePnpmCatalogs(content)); - } catch { - logger.debug({ packageFile }, `Invalid pnpm workspace YAML.`); - return null; - } + const pnpmCatalogs = pnpmCatalogsToArray(catalogs); const deps = extractPnpmCatalogDeps(pnpmCatalogs); @@ -322,10 +327,10 @@ function extractPnpmCatalogDeps( return deps; } -function parsePnpmCatalogs(content: string): PnpmCatalog[] { - const { catalog: defaultCatalogDeps, catalogs: namedCatalogs } = - parseSingleYaml(content, { customSchema: PnpmCatalogsSchema }); - +function pnpmCatalogsToArray({ + catalog: defaultCatalogDeps, + catalogs: namedCatalogs, +}: PnpmCatalogs): PnpmCatalog[] { const result = []; if (defaultCatalogDeps !== undefined) { From 35c87074d5efb2f8be0dda057c03535a09effcf0 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 19:08:43 +0200 Subject: [PATCH 45/46] chore: add explicit type annotation inside pnpmCatalogsToArray --- lib/modules/manager/npm/extract/pnpm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/manager/npm/extract/pnpm.ts b/lib/modules/manager/npm/extract/pnpm.ts index 276cf679d6d5c9..3bd596e1279e6c 100644 --- a/lib/modules/manager/npm/extract/pnpm.ts +++ b/lib/modules/manager/npm/extract/pnpm.ts @@ -331,7 +331,7 @@ function pnpmCatalogsToArray({ catalog: defaultCatalogDeps, catalogs: namedCatalogs, }: PnpmCatalogs): PnpmCatalog[] { - const result = []; + const result: PnpmCatalog[] = []; if (defaultCatalogDeps !== undefined) { result.push({ name: 'default', dependencies: defaultCatalogDeps }); From a3fb8df6abf88c5496470df70752f24676694ed4 Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Sun, 19 Jan 2025 19:57:50 +0200 Subject: [PATCH 46/46] test: explicitly test lockfile acquisition in pnpm-workspace extraction Previously, this was being tested _incidentally_, due to not mocking the filesystem calls. --- lib/modules/manager/npm/extract/pnpm.spec.ts | 61 ++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/lib/modules/manager/npm/extract/pnpm.spec.ts b/lib/modules/manager/npm/extract/pnpm.spec.ts index 4ec565245c99ce..9677ac719decc5 100644 --- a/lib/modules/manager/npm/extract/pnpm.spec.ts +++ b/lib/modules/manager/npm/extract/pnpm.spec.ts @@ -385,5 +385,66 @@ describe('modules/manager/npm/extract/pnpm', () => { ], }); }); + + it('finds relevant lockfile', async () => { + const lockfileContent = codeBlock` + lockfileVersion: '9.0' + + catalogs: + default: + react: + specifier: 18.3.1 + version: 18.3.1 + + importers: + + .: + dependencies: + react: + specifier: 'catalog:' + version: 18.3.1 + + packages: + + js-tokens@4.0.0: + resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} + + loose-envify@1.4.0: + resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} + hasBin: true + + react@18.3.1: + resolution: {integrity: sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==} + engines: {node: '>=0.10.0'} + + snapshots: + + js-tokens@4.0.0: {} + + loose-envify@1.4.0: + dependencies: + js-tokens: 4.0.0 + + react@18.3.1: + dependencies: + loose-envify: 1.4.0 + `; + fs.readLocalFile.mockResolvedValueOnce(lockfileContent); + fs.getSiblingFileName.mockReturnValueOnce('pnpm-lock.yaml'); + expect( + await extractPnpmWorkspaceFile( + { + catalog: { + react: '18.3.1', + }, + }, + 'pnpm-workspace.yaml', + ), + ).toMatchObject({ + managerData: { + pnpmShrinkwrap: 'pnpm-lock.yaml', + }, + }); + }); }); });