From 71093a9ca73a91c18d3c4af6f39941808934d4a3 Mon Sep 17 00:00:00 2001 From: Shaun Smith Date: Wed, 29 Apr 2020 20:51:45 +0100 Subject: [PATCH] fix: Reduce lodash usage - Just use JavaScript - Could not remove `lodash` completely because `isEqual` is tricky - Was tempted to switch to `lodash.isequal` package, but that will make reverting our fork harder, so leaving as-is for now --- src/core/create-from-json.ts | 9 ++++---- src/core/dep-graph.ts | 6 +++--- src/core/validate-graph.ts | 3 +-- src/legacy/index.ts | 3 +-- test/helpers.ts | 25 ++++++++++++++++----- test/legacy/from-dep-tree.test.ts | 31 +++++++++------------------ test/legacy/to-dep-tree-prune.test.ts | 1 - test/legacy/to-dep-tree.test.ts | 1 - 8 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/core/create-from-json.ts b/src/core/create-from-json.ts index 189d36a..073f190 100644 --- a/src/core/create-from-json.ts +++ b/src/core/create-from-json.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as semver from 'semver'; import * as graphlib from '@snyk/graphlib'; import * as types from './types'; @@ -93,7 +92,7 @@ function validateDepGraphData(depGraphData: DepGraphData) { nodesMap[rootNodeId].pkgId === rootPkgId, `the root node .pkgId should be "${rootPkgId}"`, ); - const pkgIds = _.keys(pkgsMap); + const pkgIds = Object.keys(pkgsMap); // NOTE: this name@version check is very strict, // we can relax it later, it just makes things easier now assert( @@ -102,11 +101,13 @@ function validateDepGraphData(depGraphData: DepGraphData) { 'pkgs ids should be name@version', ); assert( - _.values(nodesMap).filter((node) => !(node.pkgId in pkgsMap)).length === 0, + Object.values(nodesMap).filter((node) => !(node.pkgId in pkgsMap)) + .length === 0, 'some instance nodes belong to non-existing pkgIds', ); assert( - _.values(pkgsMap).filter((pkg: { name: string }) => !pkg.name).length === 0, + Object.values(pkgsMap).filter((pkg: { name: string }) => !pkg.name) + .length === 0, 'some .pkgs elements have no .name field', ); } diff --git a/src/core/dep-graph.ts b/src/core/dep-graph.ts index 5e17805..ef37fca 100644 --- a/src/core/dep-graph.ts +++ b/src/core/dep-graph.ts @@ -48,7 +48,7 @@ class DepGraphImpl implements types.DepGraphInternal { this._rootNodeId = rootNodeId; this._rootPkgId = (graph.node(rootNodeId) as GraphNode).pkgId; - this._pkgList = _.values(pkgs); + this._pkgList = Object.values(pkgs); this._depPkgsList = this._pkgList.filter((pkg) => pkg !== this.rootPkg); } @@ -213,14 +213,14 @@ class DepGraphImpl implements types.DepGraphInternal { pkgId: node.pkgId, deps, }; - if (!_.isEmpty(node.info)) { + if (node.info && Object.keys(node.info).length > 0) { elem.info = node.info; } acc.push(elem); return acc; }, []); - const pkgs: Array<{ id: string; info: types.PkgInfo }> = _.keys( + const pkgs: Array<{ id: string; info: types.PkgInfo }> = Object.keys( this._pkgs, ).map((pkgId: string) => ({ id: pkgId, diff --git a/src/core/validate-graph.ts b/src/core/validate-graph.ts index de53443..acd3f7f 100644 --- a/src/core/validate-graph.ts +++ b/src/core/validate-graph.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as graphlib from '@snyk/graphlib'; import { ValidationError } from './errors'; @@ -26,7 +25,7 @@ export function validateGraph( 'not all graph nodes are reachable from root', ); - const pkgIds = _.keys(pkgs) as string[]; + const pkgIds = Object.keys(pkgs); const pkgsWithoutInstances = pkgIds.filter( (pkgId) => !pkgNodes[pkgId] || pkgNodes[pkgId].size === 0, ); diff --git a/src/legacy/index.ts b/src/legacy/index.ts index 3d50b84..56f5ee9 100644 --- a/src/legacy/index.ts +++ b/src/legacy/index.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as crypto from 'crypto'; import * as types from '../core/types'; import { DepGraphBuilder } from '../core/builder'; @@ -95,7 +94,7 @@ async function buildGraph( const deps = depTree.dependencies || {}; // filter-out invalid null deps (shouldn't happen - but did...) - const depNames = _.keys(deps).filter((d) => !!deps[d]); + const depNames = Object.keys(deps).filter((d) => !!deps[d]); for (const depName of depNames.sort()) { const dep = deps[depName]; diff --git a/test/helpers.ts b/test/helpers.ts index fef21d7..fbd9767 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -9,7 +9,7 @@ export function loadFixture(name: string) { ); } -function depSort(a: any, b: any) { +function depCompare(a: any, b: any) { if (a.name < b.name) { return -1; } else if (a.name > b.name) { @@ -24,7 +24,9 @@ function depSort(a: any, b: any) { } export function expectSamePkgs(actual: PkgInfo[], expected: PkgInfo[]) { - return expect(actual.sort(depSort)).toEqual(expected.sort(depSort)); + actual = actual.slice().sort(depCompare); + expected = expected.slice().sort(depCompare); + return expect(actual).toEqual(expected); } export function depTreesEqual(a: any, b: any) { @@ -43,17 +45,17 @@ export function depTreesEqual(a: any, b: any) { const bDeps = b.dependencies || {}; if ( - _.keys(aDeps) + Object.keys(aDeps) .sort() .join(',') !== - _.keys(bDeps) + Object.keys(bDeps) .sort() .join(',') ) { return false; } - for (const depName of _.keys(aDeps)) { + for (const depName of Object.keys(aDeps)) { const aSubtree = aDeps[depName]; const bSubtree = bDeps[depName]; @@ -65,3 +67,16 @@ export function depTreesEqual(a: any, b: any) { return true; } + +export const sortBy = (arr: any[], p: string) => + arr.slice().sort((x: any, y: any) => { + const a = x[p]; + const b = y[p]; + if (a < b) { + return -1; + } else if (a > b) { + return 1; + } else { + return 0; + } + }); diff --git a/test/legacy/from-dep-tree.test.ts b/test/legacy/from-dep-tree.test.ts index 8aeb203..6a49fa7 100644 --- a/test/legacy/from-dep-tree.test.ts +++ b/test/legacy/from-dep-tree.test.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as depGraphLib from '../../src'; import * as types from '../../src/core/types'; import * as helpers from '../helpers'; @@ -342,25 +341,15 @@ describe('depTreeToGraph with (invalid) null dependency', () => { depTree as any, 'composer', ); - expect(_.sortBy(depGraph.getPkgs(), 'name')).toEqual( - _.sortBy( - [ - { name: 'pine', version: '4' }, - { name: 'foo', version: '1' }, - { name: 'baz', version: '3' }, - ], - 'name', - ), - ); - expect(_.sortBy(depGraph.getDepPkgs(), 'name')).toEqual( - _.sortBy( - [ - { name: 'foo', version: '1' }, - { name: 'baz', version: '3' }, - ], - 'name', - ), - ); + helpers.expectSamePkgs(depGraph.getPkgs(), [ + { name: 'pine', version: '4' }, + { name: 'foo', version: '1' }, + { name: 'baz', version: '3' }, + ]); + helpers.expectSamePkgs(depGraph.getDepPkgs(), [ + { name: 'foo', version: '1' }, + { name: 'baz', version: '3' }, + ]); }); }); @@ -444,7 +433,7 @@ describe('with labels', () => { it('getPkgNodes() returns labels', () => { let dNodes = depGraph.getPkgNodes({ name: 'd', version: '2.0.0' }); - dNodes = _.sortBy(dNodes, 'id'); + dNodes = helpers.sortBy(dNodes, 'id'); expect(dNodes[0].info.labels).toEqual({ key: 'value1' }); expect(dNodes[1].info.labels).toEqual({ key: 'value2' }); }); diff --git a/test/legacy/to-dep-tree-prune.test.ts b/test/legacy/to-dep-tree-prune.test.ts index 6611ef8..e91e389 100644 --- a/test/legacy/to-dep-tree-prune.test.ts +++ b/test/legacy/to-dep-tree-prune.test.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as depGraphLib from '../../src'; import * as helpers from '../helpers'; diff --git a/test/legacy/to-dep-tree.test.ts b/test/legacy/to-dep-tree.test.ts index 0b38288..28cef7f 100644 --- a/test/legacy/to-dep-tree.test.ts +++ b/test/legacy/to-dep-tree.test.ts @@ -1,4 +1,3 @@ -import * as _ from '@snyk/lodash'; import * as depGraphLib from '../../src'; import * as helpers from '../helpers';