From 5dae43701ab51f801c1464a68b713529dd563095 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:01:44 -0700 Subject: [PATCH 1/7] Add support for peer deps --- .../src/lib/definition-parser.ts | 3 +++ packages/definitions-parser/src/packages.ts | 11 +++++++++++ packages/definitions-parser/test/packages.test.ts | 6 ++++++ packages/definitions-parser/test/utils.ts | 2 ++ .../src/rules/no-import-of-dev-dependencies.ts | 2 +- packages/eslint-plugin/src/util.ts | 1 + packages/header-parser/src/index.ts | 1 + packages/header-parser/test/index.test.ts | 1 + packages/publisher/src/generate-packages.ts | 6 ++++++ packages/publisher/test/generate-packages.test.ts | 14 +++++++++++++- 10 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/definitions-parser/src/lib/definition-parser.ts b/packages/definitions-parser/src/lib/definition-parser.ts index 945806a637..1ffd0766ae 100644 --- a/packages/definitions-parser/src/lib/definition-parser.ts +++ b/packages/definitions-parser/src/lib/definition-parser.ts @@ -199,6 +199,7 @@ async function getPackageJsonInfoForPackage( const packageJson = fs.readJson(packageJsonName) as { readonly license?: unknown; readonly dependencies?: unknown; + readonly peerDependencies?: unknown; readonly devDependencies?: unknown; readonly imports?: unknown; readonly exports?: unknown; @@ -217,6 +218,7 @@ async function getPackageJsonInfoForPackage( const header = Array.isArray(packageJsonResult) ? undefined : packageJsonResult; const allowedDependencies = await getAllowedPackageJsonDependencies(); errors.push(...checkPackageJsonDependencies(packageJson.dependencies, packageJsonName, allowedDependencies)); + errors.push(...checkPackageJsonDependencies(packageJson.peerDependencies, packageJsonName, allowedDependencies)); errors.push( ...checkPackageJsonDependencies( packageJson.devDependencies, @@ -248,6 +250,7 @@ async function getPackageJsonInfoForPackage( license: license as License, dependencies: packageJson.dependencies as Record, devDependencies: packageJson.devDependencies as Record, + peerDependencies: packageJson.peerDependencies as Record | undefined, imports: imports as object | undefined, exports: exports as string | object | undefined, type: packageJsonType as "module" | undefined, diff --git a/packages/definitions-parser/src/packages.ts b/packages/definitions-parser/src/packages.ts index fc0b999dbe..36399d9b36 100644 --- a/packages/definitions-parser/src/packages.ts +++ b/packages/definitions-parser/src/packages.ts @@ -390,6 +390,11 @@ export interface TypingsDataRaw { */ readonly devDependencies: PackageJsonDependencies; + /** + * Packages that this package peer depends on. + */ + readonly peerDependencies?: PackageJsonDependencies; + /** * The [older] version of the library that this definition package refers to, as represented *on-disk*. * @@ -523,6 +528,9 @@ export class TypingsData extends PackageBase { get devDependencies(): PackageJsonDependencies { return this.data.devDependencies ?? {}; } + get peerDependencies(): PackageJsonDependencies { + return this.data.peerDependencies ?? {}; + } *allPackageJsonDependencies(): Iterable<[string, string]> { for (const [name, version] of Object.entries(this.dependencies)) { yield [name, version]; @@ -530,6 +538,9 @@ export class TypingsData extends PackageBase { for (const [name, version] of Object.entries(this.devDependencies)) { yield [name, version]; } + for (const [name, version] of Object.entries(this.peerDependencies)) { + yield [name, version]; + } } private _contentHash: string | undefined; diff --git a/packages/definitions-parser/test/packages.test.ts b/packages/definitions-parser/test/packages.test.ts index 73c37432e5..3674c5acbf 100644 --- a/packages/definitions-parser/test/packages.test.ts +++ b/packages/definitions-parser/test/packages.test.ts @@ -136,6 +136,9 @@ describe(TypingsData, () => { { "@types/known": "workspace:.", }, + { + "peer-dependency-1": "*", + } ); data = new TypingsData(dt.fs, versions["1.0"], true); }); @@ -164,6 +167,9 @@ describe(TypingsData, () => { expect(data.devDependencies).toEqual({ "@types/known": "workspace:.", }); + expect(data.peerDependencies).toEqual({ + "peer-dependency-1": "*", + }); expect(data.id).toEqual({ typesDirectoryName: "known", version: { diff --git a/packages/definitions-parser/test/utils.ts b/packages/definitions-parser/test/utils.ts index 6f7ecff79a..67f23bcd2c 100644 --- a/packages/definitions-parser/test/utils.ts +++ b/packages/definitions-parser/test/utils.ts @@ -12,6 +12,7 @@ export function createTypingsVersionRaw( libraryName: string, dependencies: { readonly [name: string]: string }, devDependencies: { readonly [name: string]: string }, + peerDependencies?: { readonly [name: string]: string }, ): TypingsVersionsRaw { return { "1.0": { @@ -29,6 +30,7 @@ export function createTypingsVersionRaw( license: License.MIT, dependencies, devDependencies, + peerDependencies, olderVersionDirectories: [], }, }; diff --git a/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts b/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts index 47832c3364..7c3e03cb62 100644 --- a/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts +++ b/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts @@ -39,7 +39,7 @@ const rule = createRule({ } return dep; }) - .filter((dep) => dep !== info.realName && packageJson.dependencies?.[dep] === undefined); // TODO(jakebailey): add test for this case from https://github.com/microsoft/DefinitelyTyped-tools/pull/773 + .filter((dep) => dep !== info.realName && packageJson.dependencies?.[dep] === undefined && packageJson.peerDependencies?.[dep] === undefined); // TODO(jakebailey): add test for this case from https://github.com/microsoft/DefinitelyTyped-tools/pull/773 commentsMatching(context.sourceCode, //, (ref, comment) => { if (devDeps.includes(ref)) { diff --git a/packages/eslint-plugin/src/util.ts b/packages/eslint-plugin/src/util.ts index 6ede893324..f2f64bfdd1 100644 --- a/packages/eslint-plugin/src/util.ts +++ b/packages/eslint-plugin/src/util.ts @@ -67,6 +67,7 @@ export interface PackageJSON { owners: string[]; dependencies?: Record; devDependencies?: Record; + peerDependencies?: Record; tsconfigs?: string[]; } diff --git a/packages/header-parser/src/index.ts b/packages/header-parser/src/index.ts index 0a8e0b8b48..cf6b880452 100644 --- a/packages/header-parser/src/index.ts +++ b/packages/header-parser/src/index.ts @@ -63,6 +63,7 @@ export function validatePackageJson( case "name": case "version": case "devDependencies": + case "peerDependencies": case "projects": case "minimumTypeScriptVersion": case "owners": diff --git a/packages/header-parser/test/index.test.ts b/packages/header-parser/test/index.test.ts index fae72ce10c..c10db0db2f 100644 --- a/packages/header-parser/test/index.test.ts +++ b/packages/header-parser/test/index.test.ts @@ -42,6 +42,7 @@ describe("validatePackageJson", () => { const header = { ...pkgJson, nonNpm: false, libraryMajorVersion: 18, libraryMinorVersion: 0 }; delete (header as any).dependencies; delete (header as any).devDependencies; + delete (header as any).peerDependencies; delete (header as any).private; delete (header as any).version; it("requires private: true", () => { diff --git a/packages/publisher/src/generate-packages.ts b/packages/publisher/src/generate-packages.ts index f9bd0b0a78..2125dec38a 100644 --- a/packages/publisher/src/generate-packages.ts +++ b/packages/publisher/src/generate-packages.ts @@ -142,6 +142,7 @@ export function createPackageJSON(typing: TypingsData, version: string): string }, scripts: {}, dependencies: typing.dependencies, + peerDependencies: typing.peerDependencies, typesPublisherContentHash: typing.getContentHash(), typeScriptVersion: typing.minTypeScriptVersion, nonNpm: typing.nonNpm === true ? typing.nonNpm : undefined, @@ -206,6 +207,11 @@ export function createReadme(typing: TypingsData, packageFS: FS): string { }`, ); lines.push(""); + const peerDependencies = Object.keys(typing.peerDependencies).sort(); + if (peerDependencies.length) { + lines.push(` * Peer dependencies: ${peerDependencies.map((d) => `[${d}](https://npmjs.com/package/${d})`).join(", ")}`); + lines.push(""); + } lines.push("# Credits"); const contributors = typing.contributors diff --git a/packages/publisher/test/generate-packages.test.ts b/packages/publisher/test/generate-packages.test.ts index 9915c3ddb3..dbdfaf2b29 100644 --- a/packages/publisher/test/generate-packages.test.ts +++ b/packages/publisher/test/generate-packages.test.ts @@ -32,6 +32,7 @@ function createRawPackage(license: License): TypingsDataRaw { typesVersions: [], license, dependencies: { "@types/madeira": "^1" }, + peerDependencies: { "@types/express": "*"}, devDependencies: { "@types/jquery": "workspace:." }, olderVersionDirectories: [], }; @@ -57,6 +58,7 @@ function defaultFS() { { name: "E", githubUsername: "e" }, ], dependencies: { "@types/madeira": "^1" }, + peerDependencies: { "@types/express": "*"}, devDependencies: { "@types/jquery": "workspace:." }, }, undefined, @@ -114,6 +116,13 @@ testo({ ), ); }, + readmeOnePeer() { + const dt = defaultFS(); + const typing = new TypingsData(dt.fs, createRawPackage(License.Apache20), /*isLatest*/ true); + expect(createReadme(typing, dt.pkgFS("jquery"))).toEqual( + expect.stringContaining("Peer dependencies: [@types/express](https://npmjs.com/package/@types/express)"), + ); + }, readmeContainsSingleFileDTS() { const dt = defaultFS(); const typing = new TypingsData(dt.fs, createRawPackage(License.Apache20), /*isLatest*/ true); @@ -156,7 +165,10 @@ testo({ "dependencies": { "@types/madeira": "^1" }, - "typesPublisherContentHash": "05febc04df55db2687c2ac05a291177c2f4fd90f76d679faaf1b01896fe5600c", + "peerDependencies": { + "@types/express": "*" + }, + "typesPublisherContentHash": "53300522250468c4161b10d962cac2d9d8f2cfee1b3dfef4b749a7c3ec839275", "typeScriptVersion": "4.8" }`); }, From f0723d296dd9efa7291825d21b3efcff2b96ed74 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:07:37 -0700 Subject: [PATCH 2/7] Fix more --- packages/publisher/src/calculate-versions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/publisher/src/calculate-versions.ts b/packages/publisher/src/calculate-versions.ts index 8dc219b4a6..4f663a5b9d 100644 --- a/packages/publisher/src/calculate-versions.ts +++ b/packages/publisher/src/calculate-versions.ts @@ -59,7 +59,7 @@ async function computeChangedPackages(allPackages: AllPackages, log: LoggerWithE const { version, needsPublish } = await fetchTypesPackageVersionInfo(pkg, /*publish*/ true, log); if (needsPublish) { log.info(`Need to publish: ${pkg.desc}@${version}`); - for (const name of Object.keys(pkg.dependencies)) { + for (const name of Object.keys(pkg.dependencies).concat(Object.keys(pkg.peerDependencies))) { // Assert that dependencies exist on npm. // Also checked when we install the dependencies, in dtslint-runner. From 966ecd6a37f0a75c617ab926536087e087dc453f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:18:31 -0700 Subject: [PATCH 3/7] Changeset --- .changeset/fresh-masks-retire.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/fresh-masks-retire.md diff --git a/.changeset/fresh-masks-retire.md b/.changeset/fresh-masks-retire.md new file mode 100644 index 0000000000..536750449d --- /dev/null +++ b/.changeset/fresh-masks-retire.md @@ -0,0 +1,7 @@ +--- +"@definitelytyped/definitions-parser": patch +"@definitelytyped/eslint-plugin": patch +"@definitelytyped/header-parser": patch +--- + +Add support for peerDependencies From 17b3be1080a9be65c929a6e4b8fdc1a11c4e6e1e Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:21:56 -0700 Subject: [PATCH 4/7] Bump 5.7 so I can test master --- pnpm-lock.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8dfd73a83e..cc629a4b2a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -482,7 +482,7 @@ importers: version: /typescript@5.6.0-dev.20240628 typescript-5.7: specifier: npm:typescript@~5.7.0-0 - version: /typescript@5.7.0-dev.20240822 + version: /typescript@5.7.0-dev.20240930 packages/typescript-versions: {} @@ -9929,8 +9929,8 @@ packages: hasBin: true dev: false - /typescript@5.7.0-dev.20240822: - resolution: {integrity: sha512-JL60efPDEbshbweVXX18Ic5g+9iVbf2Nc3xh3eBGbLjrt5+x6KZzB3c2d+J5PoqOzPjbFATCT2FQ3W8vEGOXTg==} + /typescript@5.7.0-dev.20240930: + resolution: {integrity: sha512-mGW96cpbwcYuc264UPqGpKBLNebFFtVEXRLPwSwxVEub9tSyMI+XGFGxsznop7pU8E1ntx4OC4cN1n5MnzO0+Q==} engines: {node: '>=14.17'} hasBin: true dev: false From 1e355d82dac6bf68fd6df992485593b69d4d2906 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:51:50 -0700 Subject: [PATCH 5/7] Complain about non-empty peer deps in mergebot --- packages/mergebot/src/pr-info.ts | 12 +++++++++++- packages/mergebot/src/urls.ts | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/mergebot/src/pr-info.ts b/packages/mergebot/src/pr-info.ts index 32ffcfb867..43800989e2 100644 --- a/packages/mergebot/src/pr-info.ts +++ b/packages/mergebot/src/pr-info.ts @@ -447,7 +447,17 @@ const configSuspicious = (async (path, newContents, oldContent const oldText = await oldContents(); return checker(text, oldText); }); -configSuspicious["package.json"] = () => undefined; +configSuspicious["package.json"] = makeChecker({}, urls.packageJson, { + parse: (text) => { + const data = JSON.parse(text); + if (!data || typeof data !== "object" || Array.isArray(data)) return data; + // Only look at peer dependencies, with the goal of making them empty. + if (data.peerDependencies) { + return { peerDependencies: data.peerDependencies }; + } + return {}; + } +}); configSuspicious[".npmignore"] = () => undefined; configSuspicious["tsconfig.json"] = makeChecker( { diff --git a/packages/mergebot/src/urls.ts b/packages/mergebot/src/urls.ts index 4dded99986..02b13c5e30 100644 --- a/packages/mergebot/src/urls.ts +++ b/packages/mergebot/src/urls.ts @@ -11,6 +11,7 @@ export const testingNewPackages = readmeLink("Adding tests to a new package"); export const definitionOwners = readmeLink("Definition Owners"); export const workflow = readmeLink("Make a pull request"); export const tsconfigJson = readmeLink("`tsconfig.json`"); +export const packageJson = readmeLink("`package.json`"); export const testsTs = readmeLink("`-tests.ts`"); export const playground = (prNum: number) => From c646c5768e22091ca9515c078074b8354bf22ea3 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:56:34 -0700 Subject: [PATCH 6/7] Add test case with peer deps --- .../src/_tests/fixtures/70751/_downloads.json | 3 + .../src/_tests/fixtures/70751/_files.json | 6 + .../src/_tests/fixtures/70751/_response.json | 149 ++++++++++++++++++ .../src/_tests/fixtures/70751/derived.json | 44 ++++++ .../src/_tests/fixtures/70751/mutations.json | 54 +++++++ .../src/_tests/fixtures/70751/result.json | 20 +++ 6 files changed, 276 insertions(+) create mode 100644 packages/mergebot/src/_tests/fixtures/70751/_downloads.json create mode 100644 packages/mergebot/src/_tests/fixtures/70751/_files.json create mode 100644 packages/mergebot/src/_tests/fixtures/70751/_response.json create mode 100644 packages/mergebot/src/_tests/fixtures/70751/derived.json create mode 100644 packages/mergebot/src/_tests/fixtures/70751/mutations.json create mode 100644 packages/mergebot/src/_tests/fixtures/70751/result.json diff --git a/packages/mergebot/src/_tests/fixtures/70751/_downloads.json b/packages/mergebot/src/_tests/fixtures/70751/_downloads.json new file mode 100644 index 0000000000..c77cdcc0e8 --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/_downloads.json @@ -0,0 +1,3 @@ +{ + "express": 102459932 +} diff --git a/packages/mergebot/src/_tests/fixtures/70751/_files.json b/packages/mergebot/src/_tests/fixtures/70751/_files.json new file mode 100644 index 0000000000..954015ac46 --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/_files.json @@ -0,0 +1,6 @@ +{ + "9856226df8d03bf135be670297bb5bf01f79625a:types/express/package.json": "{\n \"private\": true,\n \"name\": \"@types/express\",\n \"version\": \"5.0.9999\",\n \"projects\": [\n \"http://expressjs.com\"\n ],\n \"dependencies\": {\n \"@types/body-parser\": \"*\",\n \"@types/qs\": \"*\",\n \"@types/serve-static\": \"*\"\n },\n \"peerDependencies\": {\n \"@types/express-serve-static-core\": \"^5.0.0\"\n },\n \"devDependencies\": {\n \"@types/express\": \"workspace:.\",\n \"@types/node\": \"*\"\n },\n \"owners\": [\n {\n \"name\": \"Boris Yankov\",\n \"githubUsername\": \"borisyankov\"\n },\n {\n \"name\": \"China Medical University Hospital\",\n \"githubUsername\": \"CMUH\"\n },\n {\n \"name\": \"Puneet Arora\",\n \"githubUsername\": \"puneetar\"\n },\n {\n \"name\": \"Dylan Frankland\",\n \"githubUsername\": \"dfrankland\"\n }\n ]\n}\n", + "b6d1f2876fe51aed6de22ab5a293daf0e3b16ab2:types/express/package.json": "{\n \"private\": true,\n \"name\": \"@types/express\",\n \"version\": \"5.0.9999\",\n \"projects\": [\n \"http://expressjs.com\"\n ],\n \"dependencies\": {\n \"@types/body-parser\": \"*\",\n \"@types/express-serve-static-core\": \"^5.0.0\",\n \"@types/qs\": \"*\",\n \"@types/serve-static\": \"*\"\n },\n \"devDependencies\": {\n \"@types/express\": \"workspace:.\",\n \"@types/node\": \"*\"\n },\n \"owners\": [\n {\n \"name\": \"Boris Yankov\",\n \"githubUsername\": \"borisyankov\"\n },\n {\n \"name\": \"China Medical University Hospital\",\n \"githubUsername\": \"CMUH\"\n },\n {\n \"name\": \"Puneet Arora\",\n \"githubUsername\": \"puneetar\"\n },\n {\n \"name\": \"Dylan Frankland\",\n \"githubUsername\": \"dfrankland\"\n }\n ]\n}\n", + "9856226df8d03bf135be670297bb5bf01f79625a:types/express/v4/package.json": "{\n \"private\": true,\n \"name\": \"@types/express\",\n \"version\": \"4.17.9999\",\n \"projects\": [\n \"http://expressjs.com\"\n ],\n \"dependencies\": {\n \"@types/body-parser\": \"*\",\n \"@types/qs\": \"*\",\n \"@types/serve-static\": \"*\"\n },\n \"peerDependencies\": {\n \"@types/express-serve-static-core\": \"^4.17.33\"\n },\n \"devDependencies\": {\n \"@types/express\": \"workspace:.\",\n \"@types/node\": \"*\"\n },\n \"owners\": [\n {\n \"name\": \"Boris Yankov\",\n \"githubUsername\": \"borisyankov\"\n },\n {\n \"name\": \"China Medical University Hospital\",\n \"githubUsername\": \"CMUH\"\n },\n {\n \"name\": \"Puneet Arora\",\n \"githubUsername\": \"puneetar\"\n },\n {\n \"name\": \"Dylan Frankland\",\n \"githubUsername\": \"dfrankland\"\n }\n ]\n}\n", + "b6d1f2876fe51aed6de22ab5a293daf0e3b16ab2:types/express/v4/package.json": "{\n \"private\": true,\n \"name\": \"@types/express\",\n \"version\": \"4.17.9999\",\n \"projects\": [\n \"http://expressjs.com\"\n ],\n \"dependencies\": {\n \"@types/body-parser\": \"*\",\n \"@types/express-serve-static-core\": \"^4.17.33\",\n \"@types/qs\": \"*\",\n \"@types/serve-static\": \"*\"\n },\n \"devDependencies\": {\n \"@types/express\": \"workspace:.\",\n \"@types/node\": \"*\"\n },\n \"owners\": [\n {\n \"name\": \"Boris Yankov\",\n \"githubUsername\": \"borisyankov\"\n },\n {\n \"name\": \"China Medical University Hospital\",\n \"githubUsername\": \"CMUH\"\n },\n {\n \"name\": \"Puneet Arora\",\n \"githubUsername\": \"puneetar\"\n },\n {\n \"name\": \"Dylan Frankland\",\n \"githubUsername\": \"dfrankland\"\n }\n ]\n}\n" +} diff --git a/packages/mergebot/src/_tests/fixtures/70751/_response.json b/packages/mergebot/src/_tests/fixtures/70751/_response.json new file mode 100644 index 0000000000..bc30bae50c --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/_response.json @@ -0,0 +1,149 @@ +{ + "data": { + "repository": { + "id": "MDEwOlJlcG9zaXRvcnk2MDkzMzE2", + "pullRequest": { + "id": "PR_kwDOAFz6BM59K3Uu", + "title": "Use peer dep for express", + "createdAt": "2024-09-30T20:54:41Z", + "author": { + "login": "jakebailey", + "__typename": "User" + }, + "authorAssociation": "MEMBER", + "baseRef": { + "name": "master", + "__typename": "Ref" + }, + "labels": { + "nodes": [], + "__typename": "LabelConnection" + }, + "isDraft": false, + "mergeable": "MERGEABLE", + "number": 70751, + "state": "OPEN", + "headRefOid": "9856226df8d03bf135be670297bb5bf01f79625a", + "changedFiles": 2, + "additions": 6, + "deletions": 2, + "commitIds": { + "nodes": [ + { + "commit": { + "oid": "9856226df8d03bf135be670297bb5bf01f79625a", + "parents": { + "nodes": [ + { + "oid": "b6d1f2876fe51aed6de22ab5a293daf0e3b16ab2", + "__typename": "Commit" + } + ], + "__typename": "CommitConnection" + }, + "__typename": "Commit" + }, + "__typename": "PullRequestCommit" + } + ], + "__typename": "PullRequestCommitConnection" + }, + "timelineItems": { + "nodes": [], + "__typename": "PullRequestTimelineItemsConnection" + }, + "reviews": { + "nodes": [], + "__typename": "PullRequestReviewConnection" + }, + "commits": { + "totalCount": 1, + "nodes": [ + { + "commit": { + "checkSuites": { + "nodes": [ + { + "databaseId": 29037908492, + "app": { + "name": "GitHub Actions", + "__typename": "App" + }, + "conclusion": null, + "resourcePath": "/DefinitelyTyped/DefinitelyTyped/commit/9856226df8d03bf135be670297bb5bf01f79625a/checks?check_suite_id=29037908492", + "status": "IN_PROGRESS", + "url": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/9856226df8d03bf135be670297bb5bf01f79625a/checks?check_suite_id=29037908492", + "checkRuns": { + "nodes": [ + { + "title": null, + "__typename": "CheckRun" + } + ], + "__typename": "CheckRunConnection" + }, + "createdAt": "2024-09-30T20:54:45Z", + "workflowRun": { + "file": { + "path": ".github/workflows/CI.yml", + "__typename": "WorkflowRunFile" + }, + "__typename": "WorkflowRun" + }, + "__typename": "CheckSuite" + } + ], + "__typename": "CheckSuiteConnection" + }, + "status": null, + "authoredDate": "2024-09-30T20:54:04Z", + "committedDate": "2024-09-30T20:54:04Z", + "pushedDate": null, + "oid": "9856226df8d03bf135be670297bb5bf01f79625a", + "__typename": "Commit" + }, + "__typename": "PullRequestCommit" + } + ], + "__typename": "PullRequestCommitConnection" + }, + "comments": { + "totalCount": 0, + "nodes": [], + "__typename": "IssueCommentConnection" + }, + "files": { + "totalCount": 2, + "nodes": [ + { + "path": "types/express/package.json", + "additions": 3, + "deletions": 1, + "__typename": "PullRequestChangedFile" + }, + { + "path": "types/express/v4/package.json", + "additions": 3, + "deletions": 1, + "__typename": "PullRequestChangedFile" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "Mg", + "__typename": "PageInfo" + }, + "__typename": "PullRequestChangedFileConnection" + }, + "projectItems": { + "nodes": [], + "__typename": "ProjectV2ItemConnection" + }, + "__typename": "PullRequest" + }, + "__typename": "Repository" + } + }, + "loading": false, + "networkStatus": 7 +} diff --git a/packages/mergebot/src/_tests/fixtures/70751/derived.json b/packages/mergebot/src/_tests/fixtures/70751/derived.json new file mode 100644 index 0000000000..c5d045ee21 --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/derived.json @@ -0,0 +1,44 @@ +{ + "type": "info", + "now": "2024-09-30T20:55:05.622Z", + "pr_number": 70751, + "author": "jakebailey", + "headCommitOid": "9856226df8d03bf135be670297bb5bf01f79625a", + "mergeBaseOid": "b6d1f2876fe51aed6de22ab5a293daf0e3b16ab2", + "lastPushDate": "2024-09-30T20:54:41.000Z", + "lastActivityDate": "2024-09-30T20:54:41.000Z", + "hasMergeConflict": false, + "isFirstContribution": false, + "tooManyFiles": false, + "hugeChange": false, + "popularityLevel": "Critical", + "pkgInfo": [ + { + "name": "express", + "kind": "edit", + "files": [ + { + "path": "types/express/package.json", + "kind": "package-meta", + "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)" + }, + { + "path": "types/express/v4/package.json", + "kind": "package-meta", + "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)" + } + ], + "owners": [ + "borisyankov", + "CMUH", + "puneetar", + "dfrankland" + ], + "addedOwners": [], + "deletedOwners": [], + "popularityLevel": "Critical" + } + ], + "reviews": [], + "ciResult": "unknown" +} diff --git a/packages/mergebot/src/_tests/fixtures/70751/mutations.json b/packages/mergebot/src/_tests/fixtures/70751/mutations.json new file mode 100644 index 0000000000..fad3c309aa --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/mutations.json @@ -0,0 +1,54 @@ +[ + { + "mutation": "mutation ($input: AddCommentInput!) {\n addComment(input: $input) {\n __typename\n }\n}\n", + "variables": { + "input": { + "subjectId": "PR_kwDOAFz6BM59K3Uu", + "body": "@jakebailey Thank you for submitting this PR!\n\n***This is a live comment that I will keep updated.***\n\n## 1 package in this PR\n\n* `express` — [on npm](https://www.npmjs.com/package/express), [on unpkg](https://unpkg.com/browse/express@latest/)\n - Config files to check:\n - [`express/package.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files/9856226df8d03bf135be670297bb5bf01f79625a#diff-d8a8870b20691ea142d2c76db164e2732ff20bf2e1ba742a04d86e4a6adacc18): not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)\n - [`express/v4/package.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files/9856226df8d03bf135be670297bb5bf01f79625a#diff-6d96baa63d84c72911de2708233f00cd6bd6ef09dd3f91e9b4b412fa51cfd51b): not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\nYou can test the changes of this PR [in the Playground](https://www.typescriptlang.org/play/?dtPR=70751&install-plugin=playground-dt-review).\n\n## Status\n\n * ✅ No merge conflicts\n * 🕐 Continuous integration tests are still running\n * 🕐 A DT maintainer needs to approve changes that affect module config files\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ...\n" + } + } + }, + { + "mutation": "mutation ($input: AddLabelsToLabelableInput!) {\n addLabelsToLabelable(input: $input) {\n __typename\n }\n}\n", + "variables": { + "input": { + "labelIds": [ + "MDU6TGFiZWwxNjA4NjM0NDg0", + "MDU6TGFiZWwyMTU0ODE2NTQ5" + ], + "labelableId": "PR_kwDOAFz6BM59K3Uu" + } + } + }, + { + "mutation": "mutation ($input: AddProjectV2ItemByIdInput!) {\n addProjectV2ItemById(input: $input) {\n __typename\n item {\n id\n }\n }\n}\n", + "variables": { + "input": { + "contentId": "PR_kwDOAFz6BM59K3Uu", + "projectId": "PVT_kwDOADeBNM4AkH1q" + } + } + }, + { + "mutation": "mutation ($input: UpdateProjectV2ItemFieldValueInput!) {\n updateProjectV2ItemFieldValue(input: $input) {\n __typename\n }\n}\n", + "variables": { + "input": { + "itemId": "TEST", + "projectId": "PVT_kwDOADeBNM4AkH1q", + "fieldId": "PVTSSF_lADOADeBNM4AkH1qzgcYOEM", + "value": { + "singleSelectOptionId": "98236657" + } + } + } + }, + { + "mutation": "mutation ($input: AddCommentInput!) {\n addComment(input: $input) {\n __typename\n }\n}\n", + "variables": { + "input": { + "subjectId": "PR_kwDOAFz6BM59K3Uu", + "body": "🔔 @borisyankov @CMUH @puneetar @dfrankland — please [review this PR](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files) in the next few days. Be sure to explicitly select **`Approve`** or **`Request Changes`** in the GitHub UI so I know what's going on.\n" + } + } + } +] diff --git a/packages/mergebot/src/_tests/fixtures/70751/result.json b/packages/mergebot/src/_tests/fixtures/70751/result.json new file mode 100644 index 0000000000..aa12e3e035 --- /dev/null +++ b/packages/mergebot/src/_tests/fixtures/70751/result.json @@ -0,0 +1,20 @@ +{ + "projectColumn": "Waiting for Code Reviews", + "labels": [ + "Critical package", + "Check Config" + ], + "responseComments": [ + { + "tag": "welcome", + "status": "@jakebailey Thank you for submitting this PR!\n\n***This is a live comment that I will keep updated.***\n\n## 1 package in this PR\n\n* `express` — [on npm](https://www.npmjs.com/package/express), [on unpkg](https://unpkg.com/browse/express@latest/)\n - Config files to check:\n - [`express/package.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files/9856226df8d03bf135be670297bb5bf01f79625a#diff-d8a8870b20691ea142d2c76db164e2732ff20bf2e1ba742a04d86e4a6adacc18): not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)\n - [`express/v4/package.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files/9856226df8d03bf135be670297bb5bf01f79625a#diff-6d96baa63d84c72911de2708233f00cd6bd6ef09dd3f91e9b4b412fa51cfd51b): not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\nYou can test the changes of this PR [in the Playground](https://www.typescriptlang.org/play/?dtPR=70751&install-plugin=playground-dt-review).\n\n## Status\n\n * ✅ No merge conflicts\n * 🕐 Continuous integration tests are still running\n * 🕐 A DT maintainer needs to approve changes that affect module config files\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ..." + }, + { + "tag": "pinging-reviewers", + "status": "🔔 @borisyankov @CMUH @puneetar @dfrankland — please [review this PR](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70751/files) in the next few days. Be sure to explicitly select **`Approve`** or **`Request Changes`** in the GitHub UI so I know what's going on." + } + ], + "shouldClose": false, + "shouldMerge": false, + "shouldUpdateLabels": true +} From 62fc1ad0349e9ff18e5b253129fecedbe112bf8f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 30 Sep 2024 21:02:16 -0700 Subject: [PATCH 7/7] Add test for allowed deps --- packages/definitions-parser/src/mocks.ts | 19 ++++++++++- .../test/definition-parser.test.ts | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/definitions-parser/src/mocks.ts b/packages/definitions-parser/src/mocks.ts index 2531afc332..6c4caf4f90 100644 --- a/packages/definitions-parser/src/mocks.ts +++ b/packages/definitions-parser/src/mocks.ts @@ -256,6 +256,18 @@ console.log(jQuery); }), ); + const allowedDep = dt.pkgDir("allowed-dep"); + allowedDep.set("package.json", packageJson("allowed-dep", "1.0", { "prettier": "*" })); + + const nonAllowedDep = dt.pkgDir("non-allowed-dep"); + nonAllowedDep.set("package.json", packageJson("non-allowed-dep", "1.0", { "not-allowed": "*" })); + + const allowedPeerDep = dt.pkgDir("allowed-peer-dep"); + allowedPeerDep.set("package.json", packageJson("allowed-peer-dep", "1.0", {}, { "prettier": "*" })); + + const nonAllowedPeerDep = dt.pkgDir("non-allowed-peer-dep"); + nonAllowedPeerDep.set("package.json", packageJson("non-allowed-peer-dep", "1.0", {}, { "not-allowed-peer": "*" })); + return dt; } @@ -287,7 +299,7 @@ ${testNames.map((s) => " " + JSON.stringify(s)).join(",\n")} }`; } -function packageJson(packageName: string, version: string, dependencies: Record) { +function packageJson(packageName: string, version: string, dependencies: Record, peerDependencies?: Record) { return `{ "private": true, "name": "@types/${packageName}", @@ -302,6 +314,11 @@ function packageJson(packageName: string, version: string, dependencies: Record< .map(([name, version]) => ` "${name}": "${version}"`) .join(",\n")} }, + ${peerDependencies ? `"peerDependencies": { + ${Object.entries(peerDependencies) + .map(([name, version]) => ` "${name}": "${version}"`) + .join(",\n")} + },` : ""} "devDependencies": { "@types/${packageName}": "workspace:." } diff --git a/packages/definitions-parser/test/definition-parser.test.ts b/packages/definitions-parser/test/definition-parser.test.ts index c9e9141784..a15369de7c 100644 --- a/packages/definitions-parser/test/definition-parser.test.ts +++ b/packages/definitions-parser/test/definition-parser.test.ts @@ -429,4 +429,38 @@ import route = require('@ember/routing/route'); const dt = createMockDT(); return expect(getTypingInfo("wordpress__plugins", dt.fs)).resolves.toBeDefined(); }); + + it("does not error on allowed dependencies", () => { + const dt = createMockDT(); + + return expect(getTypingInfo("allowed-dep", dt.fs)).resolves.not.toHaveProperty("errors"); + }); + + it("errors on non-allowed dependencies", () => { + const dt = createMockDT(); + + return expect(getTypingInfo("non-allowed-dep", dt.fs)).resolves.toEqual({ + errors: [ + "In package.json: Dependency not-allowed not in the allowed dependencies list.\n" + + "Please make a pull request to microsoft/DefinitelyTyped-tools adding it to `packages/definitions-parser/allowedPackageJsonDependencies.txt`.", + ], + }); + }); + + it("does not error on allowed peer dependencies", () => { + const dt = createMockDT(); + + return expect(getTypingInfo("allowed-peer-dep", dt.fs)).resolves.not.toHaveProperty("errors"); + }); + + it("errors on non-allowed peer dependencies", () => { + const dt = createMockDT(); + + return expect(getTypingInfo("non-allowed-peer-dep", dt.fs)).resolves.toEqual({ + errors: [ + "In package.json: Dependency not-allowed-peer not in the allowed dependencies list.\n" + + "Please make a pull request to microsoft/DefinitelyTyped-tools adding it to `packages/definitions-parser/allowedPackageJsonDependencies.txt`.", + ], + }); + }); });