From 60accda9fb7e0db1f0167770be5888c665b5e4e6 Mon Sep 17 00:00:00 2001 From: Kelly Selden Date: Mon, 5 Jun 2023 15:51:55 -0700 Subject: [PATCH 1/3] fix: sort the result of `buildChangeGraph` --- src/array.js | 23 +++++++++++++++++++++++ src/build-change-graph.js | 3 ++- test/build-change-graph-test.js | 32 ++++++++++++++++---------------- test/changed-files-test.js | 4 ++-- test/changed-test.js | 20 ++++++++++---------- test/release-test.js | 4 ++-- 6 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 src/array.js diff --git a/src/array.js b/src/array.js new file mode 100644 index 00000000..80a89cb8 --- /dev/null +++ b/src/array.js @@ -0,0 +1,23 @@ +'use strict'; + +function sortByString(array, callback) { + return array.sort((a, b) => { + a = callback(a); + b = callback(b); + + if (a < b) + return -1; + if (a > b) + return 1; + return 0; + }); +} + +function mapByIndex(array, index) { + return array.map(item => item[index]); +} + +module.exports = { + sortByString, + mapByIndex, +}; diff --git a/src/build-change-graph.js b/src/build-change-graph.js index 4f127091..180f926d 100644 --- a/src/build-change-graph.js +++ b/src/build-change-graph.js @@ -14,6 +14,7 @@ const { getChangedReleasableFiles } = require('./releasable'); const Set = require('superset'); const { loadPackageConfig } = require('./config'); const path = require('path'); +const { sortByString, mapByIndex } = require('./array'); async function getPackageChangedFiles({ fromCommit, @@ -175,7 +176,7 @@ async function buildChangeGraph({ crawlDag(dag, packagesWithChanges); } - return Object.values(packagesWithChanges); + return mapByIndex(sortByString(Object.entries(packagesWithChanges), ([packageName]) => packageName), 1); } module.exports = buildChangeGraph; diff --git a/test/build-change-graph-test.js b/test/build-change-graph-test.js index 3703c13e..d5e43fad 100644 --- a/test/build-change-graph-test.js +++ b/test/build-change-graph-test.js @@ -949,27 +949,27 @@ describe(buildChangeGraph, function() { expect(packagesWithChanges).to.match(this.match([ { changedFiles: [ - 'packages/my-app-1/changed.txt', + 'packages/package-a/changed.txt', ], changedReleasableFiles: [ - 'packages/my-app-1/changed.txt', + 'packages/package-a/changed.txt', ], dag: this.match({ node: { - packageName: 'my-app-1', + packageName: '@scope/package-a', }, }), }, { changedFiles: [ - 'packages/package-a/changed.txt', + 'packages/my-app-1/changed.txt', ], changedReleasableFiles: [ - 'packages/package-a/changed.txt', + 'packages/my-app-1/changed.txt', ], dag: this.match({ node: { - packageName: '@scope/package-a', + packageName: 'my-app-1', }, }), }, @@ -1107,31 +1107,31 @@ describe(buildChangeGraph, function() { expect(packagesWithChanges).to.match(this.match([ { changedFiles: [ - 'packages/my-app-1/changed.txt', - 'packages/my-app-1/package.json', + 'packages/package-a/changed.txt', + 'packages/package-a/package.json', ], changedReleasableFiles: [ - 'packages/my-app-1/changed.txt', - 'packages/my-app-1/package.json', + 'packages/package-a/changed.txt', + 'packages/package-a/package.json', ], dag: this.match({ node: { - packageName: 'my-app-1', + packageName: '@scope/package-a', }, }), }, { changedFiles: [ - 'packages/package-a/changed.txt', - 'packages/package-a/package.json', + 'packages/my-app-1/changed.txt', + 'packages/my-app-1/package.json', ], changedReleasableFiles: [ - 'packages/package-a/changed.txt', - 'packages/package-a/package.json', + 'packages/my-app-1/changed.txt', + 'packages/my-app-1/package.json', ], dag: this.match({ node: { - packageName: '@scope/package-a', + packageName: 'my-app-1', }, }), }, diff --git a/test/changed-files-test.js b/test/changed-files-test.js index 775c3b46..c26db377 100644 --- a/test/changed-files-test.js +++ b/test/changed-files-test.js @@ -402,8 +402,8 @@ describe(changedFiles, function() { }); expect(_changedFiles).to.deep.equal([ - 'packages/my-app-1/changed.txt', 'packages/package-a/changed.txt', + 'packages/my-app-1/changed.txt', ]); }); @@ -505,8 +505,8 @@ describe(changedFiles, function() { }); expect(_changedFiles).to.deep.equal([ - 'packages/my-app-1/changed.txt', 'packages/package-a/changed.txt', + 'packages/my-app-1/changed.txt', 'changed', ]); }); diff --git a/test/changed-test.js b/test/changed-test.js index dc5d79ae..1bd0face 100644 --- a/test/changed-test.js +++ b/test/changed-test.js @@ -115,8 +115,8 @@ describe(changed, function() { expect(_changed).to.deep.equal([ '@scope/package-a', - 'my-app-1', '@scope/package-b', + 'my-app-1', 'root', ]); }); @@ -233,10 +233,10 @@ describe(changed, function() { }); expect(_changed).to.deep.equal([ - 'my-app-2', '@scope/package-a', - 'my-app-1', '@scope/package-b', + 'my-app-1', + 'my-app-2', 'root', ]); }); @@ -266,9 +266,9 @@ describe(changed, function() { expect(cachedChanged).to.deep.equal([ '@scope/package-a', - 'root', - 'my-app-1', '@scope/package-b', + 'my-app-1', + 'root', ]); let commit = await getCurrentCommit(tmpPath); @@ -292,9 +292,9 @@ describe(changed, function() { expect(_changed).to.deep.equal([ '@scope/package-a', - 'root', - 'my-app-1', '@scope/package-b', + 'my-app-1', + 'root', ]); _changed = await changed({ @@ -314,11 +314,11 @@ describe(changed, function() { }); expect(_changed).to.deep.equal([ - 'my-app-2', '@scope/package-a', - 'root', - 'my-app-1', '@scope/package-b', + 'my-app-1', + 'my-app-2', + 'root', ]); }); }); diff --git a/test/release-test.js b/test/release-test.js index 8c30b599..46c04e5a 100644 --- a/test/release-test.js +++ b/test/release-test.js @@ -190,7 +190,7 @@ describe(_release, function() { let lastCommitMessage = await getLastCommitMessage(tmpPath); - expect(lastCommitMessage).to.equal('chore(release): my-app@1.0.1,@scope/package-a@2.0.0,@scope/package-b@3.0.0,@scope/package-c@3.0.1'); + expect(lastCommitMessage).to.equal('chore(release): @scope/package-a@2.0.0,@scope/package-b@3.0.0,@scope/package-c@3.0.1,my-app@1.0.1'); let tags = await getTagsOnLastCommit(tmpPath); @@ -347,7 +347,7 @@ describe(_release, function() { let lastCommitMessage = await getLastCommitMessage(tmpPath); - expect(lastCommitMessage).to.equal('chore(release): my-app@0.0.1,@scope/package-a@2.0.0,@scope/package-b@3.0.0,@scope/package-c@3.0.1,root@1.0.0'); + expect(lastCommitMessage).to.equal('chore(release): @scope/package-a@2.0.0,@scope/package-b@3.0.0,@scope/package-c@3.0.1,my-app@0.0.1,root@1.0.0'); let tags = await getTagsOnLastCommit(tmpPath); From 3491965a1460a0af269f8b0527d9ad405e57e699 Mon Sep 17 00:00:00 2001 From: Kelly Selden Date: Mon, 5 Jun 2023 15:27:24 -0700 Subject: [PATCH 2/3] chore: fix ecmaVersion eslint --- .eslintrc.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 47db6748..066e09f7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -5,6 +5,10 @@ module.exports = { extends: [ 'crowdstrike-node', ], + parserOptions: { + // eslint bug, not inheriting from eslint-config-crowdstrike correctly + ecmaVersion: require('eslint-config-crowdstrike').parserOptions.ecmaVersion, + }, overrides: [ { files: 'test/**/package.json', From 16e4103bc33a74d76b23fb890e1eb9e7a6b6dd69 Mon Sep 17 00:00:00 2001 From: Kelly Selden Date: Mon, 5 Jun 2023 15:17:03 -0700 Subject: [PATCH 3/3] feat: use pMap inside buildChangeGraph This goes from 128s to 64s in my testing. --- package.json | 1 + src/build-change-graph.js | 14 ++++++++------ yarn.lock | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 7301d438..6f2de64d 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "js-yaml": "^4.0.0", "minimatch": "^5.0.0", "npm-packlist": "^6.0.0", + "p-map": "^5.5.0", "rfc6902": "^5.0.0", "semver": "7.5.1", "standard-version": "9.5.0", diff --git a/src/build-change-graph.js b/src/build-change-graph.js index 180f926d..681d3e75 100644 --- a/src/build-change-graph.js +++ b/src/build-change-graph.js @@ -73,15 +73,17 @@ async function buildChangeGraph({ return path.relative(workspaceMeta.cwd, cwd); }); - for (let _package of collectPackages(workspaceMeta)) { + const { default: pMap } = await import('p-map'); + + await pMap(collectPackages(workspaceMeta), async (_package) => { if (!_package.packageName || !_package.version) { - continue; + return; } let nextConfig = loadPackageConfig(_package.cwd); if (!nextConfig.shouldBumpVersion) { - continue; + return; } let _fromCommit; @@ -144,7 +146,7 @@ async function buildChangeGraph({ } if (!newFiles.length) { - continue; + return; } let changedReleasableFiles = await getChangedReleasableFiles({ @@ -156,7 +158,7 @@ async function buildChangeGraph({ }); if (shouldOnlyIncludeReleasable && !changedReleasableFiles.length) { - continue; + return; } let dag = buildDAG(workspaceMeta, _package.packageName); @@ -166,7 +168,7 @@ async function buildChangeGraph({ changedReleasableFiles, dag, }; - } + }); for (let { dag, changedReleasableFiles } of Object.values(packagesWithChanges)) { if (!changedReleasableFiles.length) { diff --git a/yarn.lock b/yarn.lock index 264f547d..9e8fb96c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -705,6 +705,14 @@ aggregate-error@^3.0.0: clean-stack "^2.0.0" indent-string "^4.0.0" +aggregate-error@^4.0.0: + version "4.0.1" + resolved "https://registry.npmjs.org/aggregate-error/-/aggregate-error-4.0.1.tgz#25091fe1573b9e0be892aeda15c7c66a545f758e" + integrity sha512-0poP0T7el6Vq3rstR8Mn4V/IQrpBLO6POkUSrN7RhyY+GF/InCFShQzsQ39T25gkHhLgSLByyAz+Kjb+c2L98w== + dependencies: + clean-stack "^4.0.0" + indent-string "^5.0.0" + ajv@^6.10.0, ajv@^6.12.4: version "6.12.6" resolved "https://registry.yarnpkg.com/ajv/-/ajv-6.12.6.tgz#baf5a62e802b07d977034586f8c3baf5adf26df4" @@ -1114,6 +1122,13 @@ clean-stack@^2.0.0: resolved "https://registry.yarnpkg.com/clean-stack/-/clean-stack-2.2.0.tgz#ee8472dbb129e727b31e8a10a427dee9dfe4008b" integrity sha512-4diC9HaTE+KRAMWhDhrGOECgWZxoevMc5TlkObMqNSsVU62PYzXZ/SMTjzyGAFF1YusgxGcSWTEXBhp0CPwQ1A== +clean-stack@^4.0.0: + version "4.2.0" + resolved "https://registry.npmjs.org/clean-stack/-/clean-stack-4.2.0.tgz#c464e4cde4ac789f4e0735c5d75beb49d7b30b31" + integrity sha512-LYv6XPxoyODi36Dp976riBtSY27VmFo+MKqEU9QCCWyTrdEPDog+RWA7xQWHi6Vbp61j5c4cdzzX1NidnwtUWg== + dependencies: + escape-string-regexp "5.0.0" + cli-cursor@^3.1.0: version "3.1.0" resolved "https://registry.npmjs.org/cli-cursor/-/cli-cursor-3.1.0.tgz#264305a7ae490d1d03bf0c9ba7c925d1753af307" @@ -1665,6 +1680,11 @@ escape-string-regexp@4.0.0, escape-string-regexp@^4.0.0: resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz#14ba83a5d373e3d311e5afca29cf5bfad965bf34" integrity sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA== +escape-string-regexp@5.0.0: + version "5.0.0" + resolved "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-5.0.0.tgz#4683126b500b61762f2dbebace1806e8be31b1c8" + integrity sha512-/veY75JbMK4j1yjvuUxuVsiS/hr/4iHs9FTT6cgTexxdE0Ly/glccBAkloH/DofkjRbZU3bnoj38mOmhkZ0lHw== + escape-string-regexp@^1.0.5: version "1.0.5" resolved "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4" @@ -2495,6 +2515,11 @@ indent-string@^4.0.0: resolved "https://registry.npmjs.org/indent-string/-/indent-string-4.0.0.tgz#624f8f4497d619b2d9768531d58f4122854d7251" integrity sha512-EdDDZu4A2OyIK7Lr/2zG+w5jmbuk1DVBnEwREQvBzspBJkCEbRa8GxU1lghYcaGJCnRWibjDXlq779X1/y5xwg== +indent-string@^5.0.0: + version "5.0.0" + resolved "https://registry.npmjs.org/indent-string/-/indent-string-5.0.0.tgz#4fd2980fccaf8622d14c64d694f4cf33c81951a5" + integrity sha512-m6FAo/spmsW2Ab2fU35JTYwtOKa2yAwXSwgjSv1TJzh4Mh7mC3lzAOVLBprb72XsTrgkEIsl7YrFNAiDiRhIGg== + infer-owner@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/infer-owner/-/infer-owner-1.0.4.tgz#c4cefcaa8e51051c2a40ba2ce8a3d27295af9467" @@ -3880,6 +3905,13 @@ p-map@^4.0.0: dependencies: aggregate-error "^3.0.0" +p-map@^5.5.0: + version "5.5.0" + resolved "https://registry.npmjs.org/p-map/-/p-map-5.5.0.tgz#054ca8ca778dfa4cf3f8db6638ccb5b937266715" + integrity sha512-VFqfGDHlx87K66yZrNdI4YGtD70IRyd+zSvgks6mzHPRNkoKy+9EKP4SFC77/vTTQYmRmti7dvqC+m5jBrBAcg== + dependencies: + aggregate-error "^4.0.0" + p-try@^1.0.0: version "1.0.0" resolved "https://registry.npmjs.org/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3"