From 3f7e65e1004b855ca3f8076e07f838ebbb977c90 Mon Sep 17 00:00:00 2001 From: JasonMan34 Date: Fri, 10 Nov 2023 21:30:07 +0200 Subject: [PATCH] feat(find-packages): verify format of workspace manifest file (#7273) (#7278) close #7273 --------- Co-authored-by: Itamar Zwi Co-authored-by: Zoltan Kochan --- .changeset/fifty-pans-think.md | 5 ++ .changeset/witty-scissors-smell.md | 5 ++ pnpm-lock.yaml | 35 +++++--- workspace/find-packages/package.json | 3 +- workspace/find-packages/src/index.ts | 19 +---- workspace/find-packages/tsconfig.json | 4 +- workspace/read-manifest/README.md | 13 +++ workspace/read-manifest/jest.config.js | 1 + workspace/read-manifest/package.json | 43 ++++++++++ workspace/read-manifest/src/index.ts | 83 +++++++++++++++++++ .../__fixtures__/array/pnpm-workspace.yaml | 1 + .../no-workspace-file/pkg/package.json | 4 + .../__fixtures__/null/pnpm-workspace.yaml | 1 + .../test/__fixtures__/ok/pnpm-workspace.yaml | 3 + .../pnpm-workspace.yaml | 3 + .../pnpm-workspace.yaml | 2 + .../packages-empty/pnpm-workspace.yaml | 1 + .../packages-string/pnpm-workspace.yaml | 1 + .../__fixtures__/string/pnpm-workspace.yaml | 1 + workspace/read-manifest/test/index.ts | 52 ++++++++++++ workspace/read-manifest/tsconfig.json | 20 +++++ workspace/read-manifest/tsconfig.lint.json | 8 ++ 22 files changed, 277 insertions(+), 31 deletions(-) create mode 100644 .changeset/fifty-pans-think.md create mode 100644 .changeset/witty-scissors-smell.md create mode 100644 workspace/read-manifest/README.md create mode 100644 workspace/read-manifest/jest.config.js create mode 100644 workspace/read-manifest/package.json create mode 100644 workspace/read-manifest/src/index.ts create mode 100644 workspace/read-manifest/test/__fixtures__/array/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/no-workspace-file/pkg/package.json create mode 100644 workspace/read-manifest/test/__fixtures__/null/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/ok/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/packages-contains-empty/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/packages-contains-number/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/packages-empty/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/packages-string/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/__fixtures__/string/pnpm-workspace.yaml create mode 100644 workspace/read-manifest/test/index.ts create mode 100644 workspace/read-manifest/tsconfig.json create mode 100644 workspace/read-manifest/tsconfig.lint.json diff --git a/.changeset/fifty-pans-think.md b/.changeset/fifty-pans-think.md new file mode 100644 index 0000000000..3b2f03d019 --- /dev/null +++ b/.changeset/fifty-pans-think.md @@ -0,0 +1,5 @@ +--- +"@pnpm/workspace.read-manifest": major +--- + +Initial release. diff --git a/.changeset/witty-scissors-smell.md b/.changeset/witty-scissors-smell.md new file mode 100644 index 0000000000..60007840a5 --- /dev/null +++ b/.changeset/witty-scissors-smell.md @@ -0,0 +1,5 @@ +--- +"pnpm": patch +--- + +Throw an error on invalid `pnpm-workspace.yaml` file [#7273](https://github.com/pnpm/pnpm/issues/7273). diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8396960b92..1b52c49402 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6168,9 +6168,6 @@ importers: '@pnpm/cli-utils': specifier: workspace:* version: link:../../cli/cli-utils - '@pnpm/constants': - specifier: workspace:* - version: link:../../packages/constants '@pnpm/fs.find-packages': specifier: workspace:* version: link:../../fs/find-packages @@ -6183,9 +6180,9 @@ importers: '@pnpm/util.lex-comparator': specifier: 1.0.0 version: 1.0.0 - read-yaml-file: - specifier: ^2.1.0 - version: 2.1.0 + '@pnpm/workspace.read-manifest': + specifier: workspace:* + version: link:../read-manifest devDependencies: '@pnpm/workspace.find-packages': specifier: workspace:* @@ -6229,6 +6226,22 @@ importers: specifier: 1.0.0 version: 1.0.0 + workspace/read-manifest: + dependencies: + '@pnpm/constants': + specifier: workspace:* + version: link:../../packages/constants + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error + read-yaml-file: + specifier: ^2.1.0 + version: 2.1.0 + devDependencies: + '@pnpm/workspace.read-manifest': + specifier: workspace:* + version: 'link:' + workspace/resolve-workspace-range: dependencies: semver: @@ -8991,7 +9004,7 @@ packages: /@types/byline@4.2.35: resolution: {integrity: sha512-YRfEGhvLQrA1/ixrJ95/vqin4J+hc0OIHH89rWGE2q0Tn9Jy6BhPZTiCLV1X39VJMOoq8UCUUFN+WD+gnmBjhw==} dependencies: - '@types/node': 20.8.8 + '@types/node': 16.18.59 dev: true /@types/cacache@17.0.1: @@ -9005,7 +9018,7 @@ packages: dependencies: '@types/http-cache-semantics': 4.0.3 '@types/keyv': 3.1.4 - '@types/node': 20.8.8 + '@types/node': 16.18.59 '@types/responselike': 1.0.2 /@types/cross-spawn@6.0.4: @@ -9111,7 +9124,7 @@ packages: /@types/keyv@3.1.4: resolution: {integrity: sha512-BQ5aZNSCpj7D6K2ksrRCTmKRLEpnPvWDiLPfoGyhZ++8YtiK9d/3DBKPJgry359X/P1PfruyYwvnvwFjuEiEIg==} dependencies: - '@types/node': 20.8.8 + '@types/node': 16.18.59 /@types/lodash.clonedeep@4.5.8: resolution: {integrity: sha512-I5toZLLfTvhnuAnejjVgSpBSLSC316bVURbI0sCYI0dKY3jaJgOg2arfPC6miTNnHRi/Tk/J6BB+kzT3iB5mcw==} @@ -9267,7 +9280,7 @@ packages: /@types/responselike@1.0.2: resolution: {integrity: sha512-/4YQT5Kp6HxUDb4yhRkm0bJ7TbjvTddqX7PZ5hz6qV3pxSo72f/6YPRo+Mu2DU307tm9IioO69l7uAwn5XNcFA==} dependencies: - '@types/node': 20.8.8 + '@types/node': 16.18.59 /@types/retry@0.12.4: resolution: {integrity: sha512-l1YzFLj8Y6OhLdt7HKXlz56DoEmksB7qR8KVk+MpFsS4duwnoszLgDlLxJB0vgSqtg/rAS5gmYg5Bjw2sMJ8Ew==} @@ -9664,7 +9677,7 @@ packages: resolution: {integrity: sha512-YmG+oTBCyrAoMIx5g2I9CfyurSpHyoan+9SCj7laaFKseOe3lFEyIVKvwRBQMmSt8uzh+eY5RWeQnoyyOs6AbA==} engines: {node: '>=14.15.0'} peerDependencies: - '@yarnpkg/fslib': 3.0.0-rc.25 + '@yarnpkg/fslib': ^3.0.0-rc.25 dependencies: '@types/emscripten': 1.39.9 '@yarnpkg/fslib': 3.0.0-rc.25 diff --git a/workspace/find-packages/package.json b/workspace/find-packages/package.json index 49987894ce..6376f3715e 100644 --- a/workspace/find-packages/package.json +++ b/workspace/find-packages/package.json @@ -30,11 +30,10 @@ "homepage": "https://github.com/pnpm/pnpm/blob/main/workspace/find-packages#readme", "dependencies": { "@pnpm/cli-utils": "workspace:*", - "@pnpm/constants": "workspace:*", "@pnpm/fs.find-packages": "workspace:*", "@pnpm/types": "workspace:*", "@pnpm/util.lex-comparator": "1.0.0", - "read-yaml-file": "^2.1.0" + "@pnpm/workspace.read-manifest": "workspace:*" }, "funding": "https://opencollective.com/pnpm", "devDependencies": { diff --git a/workspace/find-packages/src/index.ts b/workspace/find-packages/src/index.ts index e9962c131e..0d8e8a125c 100644 --- a/workspace/find-packages/src/index.ts +++ b/workspace/find-packages/src/index.ts @@ -1,11 +1,9 @@ -import path from 'path' import { packageIsInstallable } from '@pnpm/cli-utils' -import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants' import { type ProjectManifest, type Project, type SupportedArchitectures } from '@pnpm/types' +import { readWorkspaceManifest } from '@pnpm/workspace.read-manifest' import { lexCompare } from '@pnpm/util.lex-comparator' import { findPackages } from '@pnpm/fs.find-packages' import { logger } from '@pnpm/logger' -import readYamlFile from 'read-yaml-file' export type { Project } @@ -40,8 +38,8 @@ export async function findWorkspacePackages ( export async function findWorkspacePackagesNoCheck (workspaceRoot: string, opts?: { patterns?: string[] }): Promise { let patterns = opts?.patterns if (patterns == null) { - const packagesManifest = await requirePackagesManifest(workspaceRoot) - patterns = packagesManifest?.packages ?? undefined + const workspaceManifest = await readWorkspaceManifest(workspaceRoot) + patterns = workspaceManifest?.packages } const pkgs = await findPackages(workspaceRoot, { ignore: [ @@ -55,17 +53,6 @@ export async function findWorkspacePackagesNoCheck (workspaceRoot: string, opts? return pkgs } -async function requirePackagesManifest (dir: string): Promise<{ packages?: string[] } | null> { - try { - return await readYamlFile<{ packages?: string[] }>(path.join(dir, WORKSPACE_MANIFEST_FILENAME)) - } catch (err: any) { // eslint-disable-line - if (err['code'] === 'ENOENT') { - return null - } - throw err - } -} - type ArrayOfWorkspacePackagesToMapResult = Record>> export function arrayOfWorkspacePackagesToMap ( diff --git a/workspace/find-packages/tsconfig.json b/workspace/find-packages/tsconfig.json index 852ff77174..5890b23aa6 100644 --- a/workspace/find-packages/tsconfig.json +++ b/workspace/find-packages/tsconfig.json @@ -16,10 +16,10 @@ "path": "../../fs/find-packages" }, { - "path": "../../packages/constants" + "path": "../../packages/types" }, { - "path": "../../packages/types" + "path": "../read-manifest" } ], "composite": true diff --git a/workspace/read-manifest/README.md b/workspace/read-manifest/README.md new file mode 100644 index 0000000000..2ed2277413 --- /dev/null +++ b/workspace/read-manifest/README.md @@ -0,0 +1,13 @@ +# @pnpm/workspace.read-manifest + +> Reads a workspace manifest file + +## Install + +``` +pnpm add @pnpm/workspace.read-manifest +``` + +## LICENSE + +MIT diff --git a/workspace/read-manifest/jest.config.js b/workspace/read-manifest/jest.config.js new file mode 100644 index 0000000000..f697d83169 --- /dev/null +++ b/workspace/read-manifest/jest.config.js @@ -0,0 +1 @@ +module.exports = require('../../jest.config.js') diff --git a/workspace/read-manifest/package.json b/workspace/read-manifest/package.json new file mode 100644 index 0000000000..acb5287e03 --- /dev/null +++ b/workspace/read-manifest/package.json @@ -0,0 +1,43 @@ +{ + "name": "@pnpm/workspace.read-manifest", + "version": "0.0.0", + "description": "Reads a workspace manifest file", + "main": "lib/index.js", + "types": "lib/index.d.ts", + "files": [ + "lib", + "!*.map" + ], + "engines": { + "node": ">=16.14" + }, + "scripts": { + "lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"", + "_test": "jest", + "test": "pnpm run compile && pnpm run _test", + "prepublishOnly": "pnpm run compile", + "compile": "tsc --build && pnpm run lint --fix" + }, + "repository": "https://github.com/pnpm/pnpm/blob/main/workspace/read-manifest", + "keywords": [ + "pnpm8", + "pnpm" + ], + "license": "MIT", + "bugs": { + "url": "https://github.com/pnpm/pnpm/issues" + }, + "homepage": "https://github.com/pnpm/pnpm/blob/main/workspace/read-manifest#readme", + "dependencies": { + "@pnpm/error": "workspace:*", + "@pnpm/constants": "workspace:*", + "read-yaml-file": "^2.1.0" + }, + "funding": "https://opencollective.com/pnpm", + "devDependencies": { + "@pnpm/workspace.read-manifest": "workspace:*" + }, + "exports": { + ".": "./lib/index.js" + } +} diff --git a/workspace/read-manifest/src/index.ts b/workspace/read-manifest/src/index.ts new file mode 100644 index 0000000000..ab7d00256d --- /dev/null +++ b/workspace/read-manifest/src/index.ts @@ -0,0 +1,83 @@ +import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants' +import { PnpmError } from '@pnpm/error' +import path from 'node:path' +import readYamlFile from 'read-yaml-file' + +export interface WorkspaceManifest { + packages?: string[] +} + +export async function readWorkspaceManifest (dir: string): Promise { + const manifest = await readManifestRaw(dir) + if (validateWorkspaceManifest(manifest)) { + return manifest + } + + return undefined +} + +async function readManifestRaw (dir: string): Promise { + try { + return await readYamlFile(path.join(dir, WORKSPACE_MANIFEST_FILENAME)) + } catch (err: any) { // eslint-disable-line + // File not exists is the same as empty file (undefined) + if (err['code'] === 'ENOENT') { + return undefined + } + + // Any other error (missing perm, invalid yaml, etc.) fails the process + throw err + } +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function validateWorkspaceManifest (manifest: any): manifest is WorkspaceManifest | undefined { + if (manifest === undefined) { + // Empty manifest is ok + return true + } + + if (manifest === null) { + throw new InvalidWorkspaceManifestError('Expected object but found - null') + } + + if (typeof manifest !== 'object') { + throw new InvalidWorkspaceManifestError(`Expected object but found - ${typeof manifest}`) + } + + if (Array.isArray(manifest)) { + throw new InvalidWorkspaceManifestError('Expected object but found - array') + } + + if (Object.keys(manifest).length === 0) { + // manifest content `{}` is ok + return true + } + + if (!manifest.packages) { + throw new InvalidWorkspaceManifestError('packages field missing or empty') + } + + if (!Array.isArray(manifest.packages)) { + throw new InvalidWorkspaceManifestError('packages field is not an array') + } + + for (const pkg of manifest.packages) { + if (!pkg) { + throw new InvalidWorkspaceManifestError('Missing or empty package') + } + + const type = typeof pkg + if (type !== 'string') { + throw new InvalidWorkspaceManifestError(`Invalid package type - ${type}`) + } + } + + return true +} + +class InvalidWorkspaceManifestError extends PnpmError { + constructor (message: string) { + super('INVALID_WORKSPACE_CONFIGURATION', message) + } +} diff --git a/workspace/read-manifest/test/__fixtures__/array/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/array/pnpm-workspace.yaml new file mode 100644 index 0000000000..e97e009e51 --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/array/pnpm-workspace.yaml @@ -0,0 +1 @@ +- "pkg" \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/no-workspace-file/pkg/package.json b/workspace/read-manifest/test/__fixtures__/no-workspace-file/pkg/package.json new file mode 100644 index 0000000000..cd6c0be32b --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/no-workspace-file/pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "pkg", + "version": "1.0.0" +} diff --git a/workspace/read-manifest/test/__fixtures__/null/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/null/pnpm-workspace.yaml new file mode 100644 index 0000000000..ec747fa47d --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/null/pnpm-workspace.yaml @@ -0,0 +1 @@ +null \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/ok/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/ok/pnpm-workspace.yaml new file mode 100644 index 0000000000..c3fa16681d --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/ok/pnpm-workspace.yaml @@ -0,0 +1,3 @@ +packages: + - "packages/**" + - "types" \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/packages-contains-empty/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/packages-contains-empty/pnpm-workspace.yaml new file mode 100644 index 0000000000..377fcd30da --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/packages-contains-empty/pnpm-workspace.yaml @@ -0,0 +1,3 @@ +packages: + - "pkg" + - \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/packages-contains-number/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/packages-contains-number/pnpm-workspace.yaml new file mode 100644 index 0000000000..7e2c588e05 --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/packages-contains-number/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - 1 \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/packages-empty/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/packages-empty/pnpm-workspace.yaml new file mode 100644 index 0000000000..56124949fb --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/packages-empty/pnpm-workspace.yaml @@ -0,0 +1 @@ +packages: \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/packages-string/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/packages-string/pnpm-workspace.yaml new file mode 100644 index 0000000000..1a6ae8aeba --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/packages-string/pnpm-workspace.yaml @@ -0,0 +1 @@ +packages: "pkg" \ No newline at end of file diff --git a/workspace/read-manifest/test/__fixtures__/string/pnpm-workspace.yaml b/workspace/read-manifest/test/__fixtures__/string/pnpm-workspace.yaml new file mode 100644 index 0000000000..ea223619a0 --- /dev/null +++ b/workspace/read-manifest/test/__fixtures__/string/pnpm-workspace.yaml @@ -0,0 +1 @@ +"pkg" \ No newline at end of file diff --git a/workspace/read-manifest/test/index.ts b/workspace/read-manifest/test/index.ts new file mode 100644 index 0000000000..57043e43c0 --- /dev/null +++ b/workspace/read-manifest/test/index.ts @@ -0,0 +1,52 @@ +import { readWorkspaceManifest } from '@pnpm/workspace.read-manifest' +import path from 'node:path' + +test('readWorkspaceManifest() works with a valid workspace file', async () => { + const manifest = await readWorkspaceManifest(path.join(__dirname, '__fixtures__/ok')) + + expect(manifest).toEqual({ + packages: ['packages/**', 'types'], + }) +}) + +test('readWorkspaceManifest() throws on string content', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/string')) + ).rejects.toThrow('Expected object but found - string') +}) + +test('readWorkspaceManifest() throws on array content', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/array')) + ).rejects.toThrow('Expected object but found - array') +}) + +test('readWorkspaceManifest() throws on empty packages field', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-empty')) + ).rejects.toThrow('packages field missing or empty') +}) + +test('readWorkspaceManifest() throws on string packages field', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-string')) + ).rejects.toThrow('packages field is not an array') +}) + +test('readWorkspaceManifest() throws on empty package', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-contains-empty')) + ).rejects.toThrow('Missing or empty package') +}) + +test('readWorkspaceManifest() throws on numeric package', async () => { + await expect( + readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-contains-number')) + ).rejects.toThrow('Invalid package type - number') +}) + +test('readWorkspaceManifest() works when no workspace file is present', async () => { + const manifest = await readWorkspaceManifest(path.join(__dirname, '__fixtures__/no-workspace-file')) + + expect(manifest).toBeUndefined() +}) \ No newline at end of file diff --git a/workspace/read-manifest/tsconfig.json b/workspace/read-manifest/tsconfig.json new file mode 100644 index 0000000000..5e22b12e65 --- /dev/null +++ b/workspace/read-manifest/tsconfig.json @@ -0,0 +1,20 @@ +{ + "extends": "@pnpm/tsconfig", + "compilerOptions": { + "outDir": "lib", + "rootDir": "src" + }, + "include": [ + "src/**/*.ts", + "../../__typings__/**/*.d.ts" + ], + "references": [ + { + "path": "../../packages/constants" + }, + { + "path": "../../packages/error" + } + ], + "composite": true +} diff --git a/workspace/read-manifest/tsconfig.lint.json b/workspace/read-manifest/tsconfig.lint.json new file mode 100644 index 0000000000..1bbe711971 --- /dev/null +++ b/workspace/read-manifest/tsconfig.lint.json @@ -0,0 +1,8 @@ +{ + "extends": "./tsconfig.json", + "include": [ + "src/**/*.ts", + "test/**/*.ts", + "../../__typings__/**/*.d.ts" + ] +}