From 7328e66d4d08c2192eac93b22aad6228b4aa18e5 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 14:06:11 +0100 Subject: [PATCH 1/8] chore: add test step that makes sure no external/builtin dependencies --- .gitignore | 1 + package.json | 5 ++++- rollup.config.js | 22 ++++++++++++++++++++++ test/map.js | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 rollup.config.js diff --git a/.gitignore b/.gitignore index 3b94e235..71430472 100644 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,4 @@ !/SECURITY.md !/tap-snapshots/ !/test/ +!/rollup.config.js diff --git a/package.json b/package.json index c4fca044..873f6bc3 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,9 @@ "description": "The semantic version parser used by npm.", "main": "index.js", "scripts": { - "test": "tap", + "test": "tap && npm run check-self-contained", "snap": "tap", + "check-self-contained": "rollup -c --silent > /dev/null", "lint": "eslint \"**/*.js\"", "postlint": "template-oss-check", "lintfix": "npm run lint -- --fix", @@ -15,6 +16,8 @@ "devDependencies": { "@npmcli/eslint-config": "^4.0.0", "@npmcli/template-oss": "4.14.1", + "@rollup/plugin-commonjs": "^24.1.0", + "rollup": "^3.21.5", "tap": "^16.0.0" }, "license": "ISC", diff --git a/rollup.config.js b/rollup.config.js new file mode 100644 index 00000000..e298738b --- /dev/null +++ b/rollup.config.js @@ -0,0 +1,22 @@ +const fs = require('fs') +const commonjs = require('@rollup/plugin-commonjs') + +const pkgJson = JSON.parse(fs.readFileSync('./package.json', 'utf-8')) + +module.exports = { + input: pkgJson.main, + plugins: [ + commonjs({ + strictRequires: true, + ignoreTryCatch: true, + }), + ], + external: Object.keys(pkgJson.dependencies), + onwarn: (e) => { + if (e.code === 'CIRCULAR_DEPENDENCY') { + return + } + + throw new Error(e) + }, +} diff --git a/test/map.js b/test/map.js index aded2454..52d5408f 100644 --- a/test/map.js +++ b/test/map.js @@ -11,6 +11,7 @@ const ignore = [ 'tap-snapshots', 'test', 'fixtures', + 'rollup.config.js', ] const { statSync, readdirSync } = require('fs') From d0a105ec65a4237b0509384efccdf04d5a9925e5 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 14:07:11 +0100 Subject: [PATCH 2/8] fix: remove dependency on node builtin --- classes/semver.js | 2 +- test/functions/parse.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/semver.js b/classes/semver.js index 25ee889d..ea62fa0d 100644 --- a/classes/semver.js +++ b/classes/semver.js @@ -16,7 +16,7 @@ class SemVer { version = version.version } } else if (typeof version !== 'string') { - throw new TypeError(`Invalid Version: ${require('util').inspect(version)}`) + throw new TypeError(`Invalid Version: ${version}`) } if (version.length > MAX_LENGTH) { diff --git a/test/functions/parse.js b/test/functions/parse.js index f3ca4cd8..b382ddcf 100644 --- a/test/functions/parse.js +++ b/test/functions/parse.js @@ -20,7 +20,7 @@ t.test('throw errors if asked to', t => { parse([], null, true) }, { name: 'TypeError', - message: 'Invalid Version: []', + message: 'Invalid Version: ', }) t.end() }) From 3cfdf1e9ca3cdeb13be70be24246c4968f150c03 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 15:16:16 +0100 Subject: [PATCH 3/8] chore: use eslint plugin for preventing externals/builtins --- .eslintrc.local.js | 27 +++++++++++++++++++++++++++ .gitignore | 1 - package.json | 6 ++---- rollup.config.js | 22 ---------------------- test/map.js | 2 +- 5 files changed, 30 insertions(+), 28 deletions(-) create mode 100644 .eslintrc.local.js delete mode 100644 rollup.config.js diff --git a/.eslintrc.local.js b/.eslintrc.local.js new file mode 100644 index 00000000..071d2379 --- /dev/null +++ b/.eslintrc.local.js @@ -0,0 +1,27 @@ +'use strict' + +module.exports = { + plugins: ['import'], + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { + devDependencies: false, + optionalDependencies: false, + peerDependencies: false, + bundledDependencies: false, + includeInternal: true, + }, + ], + 'import/no-nodejs-modules': ['error'], + }, + overrides: [ + { + files: ['**/test/**', '.eslintrc.js', '.eslintrc.local.js'], + rules: { + 'import/no-extraneous-dependencies': 0, + 'import/no-nodejs-modules': 0, + }, + }, + ], +} diff --git a/.gitignore b/.gitignore index 71430472..3b94e235 100644 --- a/.gitignore +++ b/.gitignore @@ -33,4 +33,3 @@ !/SECURITY.md !/tap-snapshots/ !/test/ -!/rollup.config.js diff --git a/package.json b/package.json index 873f6bc3..f2c4963f 100644 --- a/package.json +++ b/package.json @@ -4,9 +4,8 @@ "description": "The semantic version parser used by npm.", "main": "index.js", "scripts": { - "test": "tap && npm run check-self-contained", + "test": "tap", "snap": "tap", - "check-self-contained": "rollup -c --silent > /dev/null", "lint": "eslint \"**/*.js\"", "postlint": "template-oss-check", "lintfix": "npm run lint -- --fix", @@ -16,8 +15,7 @@ "devDependencies": { "@npmcli/eslint-config": "^4.0.0", "@npmcli/template-oss": "4.14.1", - "@rollup/plugin-commonjs": "^24.1.0", - "rollup": "^3.21.5", + "eslint-plugin-import": "^2.27.5", "tap": "^16.0.0" }, "license": "ISC", diff --git a/rollup.config.js b/rollup.config.js deleted file mode 100644 index e298738b..00000000 --- a/rollup.config.js +++ /dev/null @@ -1,22 +0,0 @@ -const fs = require('fs') -const commonjs = require('@rollup/plugin-commonjs') - -const pkgJson = JSON.parse(fs.readFileSync('./package.json', 'utf-8')) - -module.exports = { - input: pkgJson.main, - plugins: [ - commonjs({ - strictRequires: true, - ignoreTryCatch: true, - }), - ], - external: Object.keys(pkgJson.dependencies), - onwarn: (e) => { - if (e.code === 'CIRCULAR_DEPENDENCY') { - return - } - - throw new Error(e) - }, -} diff --git a/test/map.js b/test/map.js index 52d5408f..8179e71e 100644 --- a/test/map.js +++ b/test/map.js @@ -6,12 +6,12 @@ const ignore = [ '.github', '.commitlintrc.js', '.eslintrc.js', + '.eslintrc.local.js', 'node_modules', 'coverage', 'tap-snapshots', 'test', 'fixtures', - 'rollup.config.js', ] const { statSync, readdirSync } = require('fs') From 5019bc5b960f6fea06faa87d54052731a63fcbe9 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 15:38:57 +0100 Subject: [PATCH 4/8] fix: tweak error message when input is wrong type --- classes/semver.js | 2 +- test/classes/semver.js | 144 ++++++++++++++++++++++------------------ test/functions/parse.js | 2 +- 3 files changed, 81 insertions(+), 67 deletions(-) diff --git a/classes/semver.js b/classes/semver.js index ea62fa0d..99dbe82d 100644 --- a/classes/semver.js +++ b/classes/semver.js @@ -16,7 +16,7 @@ class SemVer { version = version.version } } else if (typeof version !== 'string') { - throw new TypeError(`Invalid Version: ${version}`) + throw new TypeError(`Invalid version. Must be a string. Got type "${typeof version}".`) } if (version.length > MAX_LENGTH) { diff --git a/test/classes/semver.js b/test/classes/semver.js index 4556434c..224e1b36 100644 --- a/test/classes/semver.js +++ b/test/classes/semver.js @@ -5,52 +5,60 @@ const comparisons = require('../fixtures/comparisons.js') const equality = require('../fixtures/equality.js') const invalidVersions = require('../fixtures/invalid-versions') -test('comparisons', t => { +test('comparisons', (t) => { t.plan(comparisons.length) - comparisons.forEach(([v0, v1, opt]) => t.test(`${v0} ${v1}`, t => { - const s0 = new SemVer(v0, opt) - const s1 = new SemVer(v1, opt) - t.equal(s0.compare(s1), 1) - t.equal(s0.compare(v1), 1) - t.equal(s1.compare(s0), -1) - t.equal(s1.compare(v0), -1) - t.equal(s0.compare(v0), 0) - t.equal(s1.compare(v1), 0) - t.end() - })) + comparisons.forEach(([v0, v1, opt]) => + t.test(`${v0} ${v1}`, (t) => { + const s0 = new SemVer(v0, opt) + const s1 = new SemVer(v1, opt) + t.equal(s0.compare(s1), 1) + t.equal(s0.compare(v1), 1) + t.equal(s1.compare(s0), -1) + t.equal(s1.compare(v0), -1) + t.equal(s0.compare(v0), 0) + t.equal(s1.compare(v1), 0) + t.end() + }) + ) }) -test('equality', t => { +test('equality', (t) => { t.plan(equality.length) - equality.forEach(([v0, v1, loose]) => t.test(`${v0} ${v1} ${loose}`, t => { - const s0 = new SemVer(v0, loose) - const s1 = new SemVer(v1, loose) - t.equal(s0.compare(s1), 0) - t.equal(s1.compare(s0), 0) - t.equal(s0.compare(v1), 0) - t.equal(s1.compare(v0), 0) - t.equal(s0.compare(s0), 0) - t.equal(s1.compare(s1), 0) - t.equal(s0.comparePre(s1), 0, 'comparePre just to hit that code path') - t.end() - })) + equality.forEach(([v0, v1, loose]) => + t.test(`${v0} ${v1} ${loose}`, (t) => { + const s0 = new SemVer(v0, loose) + const s1 = new SemVer(v1, loose) + t.equal(s0.compare(s1), 0) + t.equal(s1.compare(s0), 0) + t.equal(s0.compare(v1), 0) + t.equal(s1.compare(v0), 0) + t.equal(s0.compare(s0), 0) + t.equal(s1.compare(s1), 0) + t.equal(s0.comparePre(s1), 0, 'comparePre just to hit that code path') + t.end() + }) + ) }) -test('toString equals parsed version', t => { +test('toString equals parsed version', (t) => { t.equal(String(new SemVer('v1.2.3')), '1.2.3') t.end() }) -test('throws when presented with garbage', t => { +test('throws when presented with garbage', (t) => { t.plan(invalidVersions.length) invalidVersions.forEach(([v, msg, opts]) => - t.throws(() => new SemVer(v, opts), msg)) + t.throws(() => new SemVer(v, opts), msg) + ) }) -test('return SemVer arg to ctor if options match', t => { +test('return SemVer arg to ctor if options match', (t) => { const s = new SemVer('1.2.3', { loose: true, includePrerelease: true }) - t.equal(new SemVer(s, { loose: true, includePrerelease: true }), s, - 'get same object when options match') + t.equal( + new SemVer(s, { loose: true, includePrerelease: true }), + s, + 'get same object when options match' + ) t.not(new SemVer(s), s, 'get new object when options match') t.end() }) @@ -62,37 +70,39 @@ test('really big numeric prerelease value', (t) => { }) test('invalid version numbers', (t) => { - ['1.2.3.4', - 'NOT VALID', - 1.2, - null, - 'Infinity.NaN.Infinity', - ].forEach((v) => { - t.throws(() => { - new SemVer(v) // eslint-disable-line no-new - }, { name: 'TypeError', message: `Invalid Version: ${v}` }) + ['1.2.3.4', 'NOT VALID', 1.2, null, 'Infinity.NaN.Infinity'].forEach((v) => { + t.throws( + () => { + new SemVer(v) // eslint-disable-line no-new + }, + { + name: 'TypeError', + message: + typeof v === 'string' + ? `Invalid Version: ${v}` + : `Invalid version. Must be a string. Got type "${typeof v}".`, + } + ) }) t.end() }) -test('incrementing', t => { +test('incrementing', (t) => { t.plan(increments.length) - increments.forEach(([ - version, - inc, - expect, - options, - id, - base, - ]) => t.test(`${version} ${inc} ${id || ''}`.trim(), t => { - t.plan(1) - if (expect === null) { - t.throws(() => new SemVer(version, options).inc(inc, id, base)) - } else { - t.equal(new SemVer(version, options).inc(inc, id, base).version, expect) - } - })) + increments.forEach(([version, inc, expect, options, id, base]) => + t.test(`${version} ${inc} ${id || ''}`.trim(), (t) => { + t.plan(1) + if (expect === null) { + t.throws(() => new SemVer(version, options).inc(inc, id, base)) + } else { + t.equal( + new SemVer(version, options).inc(inc, id, base).version, + expect + ) + } + }) + ) }) test('compare main vs pre', (t) => { @@ -113,15 +123,19 @@ test('compare main vs pre', (t) => { }) test('invalid version numbers', (t) => { - ['1.2.3.4', - 'NOT VALID', - 1.2, - null, - 'Infinity.NaN.Infinity', - ].forEach((v) => { - t.throws(() => { - new SemVer(v) // eslint-disable-line no-new - }, { name: 'TypeError', message: `Invalid Version: ${v}` }) + ['1.2.3.4', 'NOT VALID', 1.2, null, 'Infinity.NaN.Infinity'].forEach((v) => { + t.throws( + () => { + new SemVer(v) // eslint-disable-line no-new + }, + { + name: 'TypeError', + message: + typeof v === 'string' + ? `Invalid Version: ${v}` + : `Invalid version. Must be a string. Got type "${typeof v}".`, + } + ) }) t.end() diff --git a/test/functions/parse.js b/test/functions/parse.js index b382ddcf..dd091e94 100644 --- a/test/functions/parse.js +++ b/test/functions/parse.js @@ -20,7 +20,7 @@ t.test('throw errors if asked to', t => { parse([], null, true) }, { name: 'TypeError', - message: 'Invalid Version: ', + message: 'Invalid version. Must be a string. Got type "object".', }) t.end() }) From 7acab3450593150d847633341bd05ef48c90880f Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 15:45:22 +0100 Subject: [PATCH 5/8] fix: don't specify eslint plugin in config because already there from base config --- .eslintrc.local.js | 1 - package.json | 1 - 2 files changed, 2 deletions(-) diff --git a/.eslintrc.local.js b/.eslintrc.local.js index 071d2379..1a257b31 100644 --- a/.eslintrc.local.js +++ b/.eslintrc.local.js @@ -1,7 +1,6 @@ 'use strict' module.exports = { - plugins: ['import'], rules: { 'import/no-extraneous-dependencies': [ 'error', diff --git a/package.json b/package.json index f2c4963f..c4fca044 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,6 @@ "devDependencies": { "@npmcli/eslint-config": "^4.0.0", "@npmcli/template-oss": "4.14.1", - "eslint-plugin-import": "^2.27.5", "tap": "^16.0.0" }, "license": "ISC", From 6f3bfbf7e0fc6b60f39843023a874348e46c96d5 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 5 May 2023 16:20:56 +0100 Subject: [PATCH 6/8] chore: undo unneeded formatting changes --- test/classes/semver.js | 100 +++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 53 deletions(-) diff --git a/test/classes/semver.js b/test/classes/semver.js index 224e1b36..6be0ac8f 100644 --- a/test/classes/semver.js +++ b/test/classes/semver.js @@ -5,60 +5,52 @@ const comparisons = require('../fixtures/comparisons.js') const equality = require('../fixtures/equality.js') const invalidVersions = require('../fixtures/invalid-versions') -test('comparisons', (t) => { +test('comparisons', t => { t.plan(comparisons.length) - comparisons.forEach(([v0, v1, opt]) => - t.test(`${v0} ${v1}`, (t) => { - const s0 = new SemVer(v0, opt) - const s1 = new SemVer(v1, opt) - t.equal(s0.compare(s1), 1) - t.equal(s0.compare(v1), 1) - t.equal(s1.compare(s0), -1) - t.equal(s1.compare(v0), -1) - t.equal(s0.compare(v0), 0) - t.equal(s1.compare(v1), 0) - t.end() - }) - ) + comparisons.forEach(([v0, v1, opt]) => t.test(`${v0} ${v1}`, t => { + const s0 = new SemVer(v0, opt) + const s1 = new SemVer(v1, opt) + t.equal(s0.compare(s1), 1) + t.equal(s0.compare(v1), 1) + t.equal(s1.compare(s0), -1) + t.equal(s1.compare(v0), -1) + t.equal(s0.compare(v0), 0) + t.equal(s1.compare(v1), 0) + t.end() + })) }) -test('equality', (t) => { +test('equality', t => { t.plan(equality.length) - equality.forEach(([v0, v1, loose]) => - t.test(`${v0} ${v1} ${loose}`, (t) => { - const s0 = new SemVer(v0, loose) - const s1 = new SemVer(v1, loose) - t.equal(s0.compare(s1), 0) - t.equal(s1.compare(s0), 0) - t.equal(s0.compare(v1), 0) - t.equal(s1.compare(v0), 0) - t.equal(s0.compare(s0), 0) - t.equal(s1.compare(s1), 0) - t.equal(s0.comparePre(s1), 0, 'comparePre just to hit that code path') - t.end() - }) - ) + equality.forEach(([v0, v1, loose]) => t.test(`${v0} ${v1} ${loose}`, t => { + const s0 = new SemVer(v0, loose) + const s1 = new SemVer(v1, loose) + t.equal(s0.compare(s1), 0) + t.equal(s1.compare(s0), 0) + t.equal(s0.compare(v1), 0) + t.equal(s1.compare(v0), 0) + t.equal(s0.compare(s0), 0) + t.equal(s1.compare(s1), 0) + t.equal(s0.comparePre(s1), 0, 'comparePre just to hit that code path') + t.end() + })) }) -test('toString equals parsed version', (t) => { +test('toString equals parsed version', t => { t.equal(String(new SemVer('v1.2.3')), '1.2.3') t.end() }) -test('throws when presented with garbage', (t) => { +test('throws when presented with garbage', t => { t.plan(invalidVersions.length) invalidVersions.forEach(([v, msg, opts]) => - t.throws(() => new SemVer(v, opts), msg) - ) + t.throws(() => new SemVer(v, opts), msg)) }) -test('return SemVer arg to ctor if options match', (t) => { +test('return SemVer arg to ctor if options match', t => { const s = new SemVer('1.2.3', { loose: true, includePrerelease: true }) - t.equal( - new SemVer(s, { loose: true, includePrerelease: true }), - s, - 'get same object when options match' - ) + t.equal(new SemVer(s, { loose: true, includePrerelease: true }), s, + 'get same object when options match') t.not(new SemVer(s), s, 'get new object when options match') t.end() }) @@ -88,21 +80,23 @@ test('invalid version numbers', (t) => { t.end() }) -test('incrementing', (t) => { +test('incrementing', t => { t.plan(increments.length) - increments.forEach(([version, inc, expect, options, id, base]) => - t.test(`${version} ${inc} ${id || ''}`.trim(), (t) => { - t.plan(1) - if (expect === null) { - t.throws(() => new SemVer(version, options).inc(inc, id, base)) - } else { - t.equal( - new SemVer(version, options).inc(inc, id, base).version, - expect - ) - } - }) - ) + increments.forEach(([ + version, + inc, + expect, + options, + id, + base, + ]) => t.test(`${version} ${inc} ${id || ''}`.trim(), t => { + t.plan(1) + if (expect === null) { + t.throws(() => new SemVer(version, options).inc(inc, id, base)) + } else { + t.equal(new SemVer(version, options).inc(inc, id, base).version, expect) + } + })) }) test('compare main vs pre', (t) => { From 9a3bd04b5f5468397f1969a5b462ebebe50bb2ef Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 12 May 2023 10:12:17 +0100 Subject: [PATCH 7/8] chore: only add the `devDependencies` option to be consistent with base config --- .eslintrc.local.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.eslintrc.local.js b/.eslintrc.local.js index 1a257b31..566a162c 100644 --- a/.eslintrc.local.js +++ b/.eslintrc.local.js @@ -6,10 +6,6 @@ module.exports = { 'error', { devDependencies: false, - optionalDependencies: false, - peerDependencies: false, - bundledDependencies: false, - includeInternal: true, }, ], 'import/no-nodejs-modules': ['error'], From b453496951918a489d659e9e7ec6fe8d3f64032a Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 12 May 2023 16:23:58 +0100 Subject: [PATCH 8/8] target specific dirs --- .eslintrc.local.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/.eslintrc.local.js b/.eslintrc.local.js index 566a162c..5b7c98ea 100644 --- a/.eslintrc.local.js +++ b/.eslintrc.local.js @@ -1,21 +1,17 @@ 'use strict' module.exports = { - rules: { - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: false, - }, - ], - 'import/no-nodejs-modules': ['error'], - }, overrides: [ { - files: ['**/test/**', '.eslintrc.js', '.eslintrc.local.js'], + files: ['bin/**', 'classes/**', 'functions/**', 'internal/**', 'ranges/**'], rules: { - 'import/no-extraneous-dependencies': 0, - 'import/no-nodejs-modules': 0, + 'import/no-extraneous-dependencies': [ + 'error', + { + devDependencies: false, + }, + ], + 'import/no-nodejs-modules': ['error'], }, }, ],