BREAKING: rewrite API

Syncs APIs with changes proposed in the `npm diff` PR. ref: npm/cli#1319
npm · Jan 23, 2021 · 7737011 · 7737011
1 parent 02befe1
commit 7737011
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 166 deletions.
diff --git a/README.md b/README.md
@@ -20,10 +20,10 @@ The registry diff lib.
 ```js
 const libdiff = require('libnpmdiff')
 
-const patch = await libdiff({
-  a: 'abbrev@1.1.0',
-  b: 'abbrev@1.1.1'
-})
+const patch = await libdiff([
+  'abbrev@1.1.0',
+  'abbrev@1.1.1'
+])
 console.log(
   patch
 )
@@ -70,27 +70,27 @@ Happy hacking!
 
 ### API
 
-#### `> libnpmdif({ a, b }, [opts]) -> Promise<String>`
+#### `> libnpmdif([ a, b ], [opts]) -> Promise<String>`
 
 Fetches the registry tarballs and compare files between a spec `a` and spec `b`. **npm** spec types are usually described in `<pkg-name>@<version>` form but multiple other types are alsos supported, for more info on valid specs take a look at [`npm-package-arg`](https://github.com/npm/npm-package-arg).
 
-If only spec `a` is provided, then it's going to try and compare that specified spec against the current project directory in the local file system (cwd may be set via `opts.prefix` option).
-
 **Options**:
 
 - `color <Boolean>`: Should add ANSI colors to string output? Defaults to `false`.
 - `tagVersionPrefix <Sring>`: What prefix should be used to define version numbers. Defaults to `v`
-- `prefix <String>`: The path to use as current working directory if trying to read from local file system when using only a single comparison parameter `a`. Defaults to `.`.
+- `diffContext <Number>`: How many lines of code to print before/after each diff. Defaults to `3`.
+- `diffFiles <Array<String>>`: If set only prints patches for the files listed in this array (also accepts globs). Defaults to `undefined`.
+- `diffIgnoreWhitespace <Boolean>`: Whether or not should ignore changes in whitespace (very useful to avoid indentation changes extra diff lines). Defaults to `false`.
+- `diffNameOnly <Boolean>`: Prints only file names and no patch diffs. Defaults to `false`.
+- `diffNoPrefix <Boolean>`: If true then skips printing any prefixes in filenames. Defaults to `false`.
+- `diffSrcPrefix <String>`: Prefix to be used in the filenames from `a`. Defaults to `a/`.
+- `diffDstPrefix <String>`: Prefix to be used in the filenames from `b`. Defaults to `b/`.
+- `diffText <Boolean>`: Should treat all files as text and try to print diff for binary files. Defaults to `false`.
 - ...`cache`, `registry` and other common options accepted by [pacote](https://github.com/npm/pacote#options)
-- `diffOpts <Object>`: Object containing extra options on how to format the diff patch output:
-  - `context <Number>`: How many lines of code to print before/after each diff. Defaults to `3`.
-  - `files <Array<String>>`: If set only prints patches for the files listed in this array (also accepts globs). Defaults to `undefined`.
-  - `ignoreWhitespace <Boolean>`: Whether or not should ignore changes in whitespace (very useful to avoid indentation changes extra diff lines). Defaults to `false`.
-  - `nameOnly <Boolean>`: Prints only file names and no patch diffs. Defaults to `false`.
-  - `noPrefix <Boolean>`: If true then skips printing any prefixes in filenames. Defaults to `false`.
-  - `srcPrefix <String>`: Prefix to be used in the filenames from `a`. Defaults to `a/`.
-  - `dstPrefix <String>`: Prefix to be used in the filenames from `b`. Defaults to `b/`.
-  - `text <Boolean>`: Should treat all files as text and try to print diff for binary files. Defaults to `false`.
+
+Returns a `Promise` that fullfils with a `String` containing the resulting patch diffs.
+
+Throws an error if either `a` or `b` are missing or if trying to diff more than two specs.
 
 ## LICENSE
 

diff --git a/index.js b/index.js
@@ -3,17 +3,20 @@ const pacote = require('pacote')
 const formatDiff = require('./lib/format-diff.js')
 const untar = require('./lib/untar.js')
 
+const argsError = () =>
+  Object.assign(
+    new TypeError('libnpmdiff needs two arguments to compare'),
+    { code: 'EDIFFARGS' }
+  )
 const diff = async (specs, opts = {}) => {
-  const { prefix: path } = opts
-
-  const aManifest = await pacote.manifest(specs.a, opts)
-
-  // when using a single argument the spec to compare from is going to be
-  // figured out from reading the current location package
-  if (!specs.b)
-    specs.b = `file:${path || '.'}`
-
-  const bManifest = await pacote.manifest(specs.b, opts)
+  if (specs.length !== 2)
+    throw argsError()
+
+  const [
+    aManifest,
+    bManifest,
+  ] =
+    await Promise.all(specs.map(spec => pacote.manifest(spec, opts)))
 
   const versions = {
     a: aManifest.version,

diff --git a/lib/format-diff.js b/lib/format-diff.js
@@ -7,9 +7,8 @@ const shouldPrintPatch = require('./should-print-patch.js')
 
 const formatDiff = ({ files, opts = {}, refs, versions }) => {
   let res = ''
-  const diffOpts = opts.diffOpts || {}
-  const srcPrefix = diffOpts.noPrefix ? '' : diffOpts.srcPrefix || 'a/'
-  const dstPrefix = diffOpts.noPrefix ? '' : diffOpts.dstPrefix || 'b/'
+  const srcPrefix = opts.diffNoPrefix ? '' : opts.diffSrcPrefix || 'a/'
+  const dstPrefix = opts.diffNoPrefix ? '' : opts.diffDstPrefix || 'b/'
 
   for (const filename of files.values()) {
     const names = {
@@ -34,7 +33,7 @@ const formatDiff = ({ files, opts = {}, refs, versions }) => {
     if (contents.a === contents.b && modes.a === modes.b)
       continue
 
-    if (diffOpts.nameOnly) {
+    if (opts.diffNameOnly) {
       res += `${filename}${EOL}`
       continue
     }
@@ -71,8 +70,8 @@ const formatDiff = ({ files, opts = {}, refs, versions }) => {
         '',
         '',
         {
-          context: diffOpts.context || 3,
-          ignoreWhitespace: diffOpts.ignoreWhitespace,
+          context: opts.diffContext || 3,
+          ignoreWhitespace: opts.diffIgnoreWhitespace,
         }
       ).replace(
         '===================================================================\n',

diff --git a/lib/should-print-patch.js b/lib/should-print-patch.js
@@ -5,8 +5,7 @@ const binaryExtensions = require('binary-extensions')
 // we should try to print patches as long as the
 // extension is not identified as binary files
 const shouldPrintPatch = (path, opts = {}) => {
-  const { text } = opts.diffOpts || {}
-  if (text)
+  if (opts.diffText)
     return true
 
   const filename = basename(path)

diff --git a/lib/untar.js b/lib/untar.js
@@ -51,9 +51,8 @@ const readTarballs = async (tarballs, opts = {}) => {
   const files = new Set()
   const refs = new Map()
   const arr = [].concat(tarballs)
-  const { files: _files } = opts.diffOpts || {}
 
-  const filterFiles = _files || []
+  const filterFiles = opts.diffFiles || []
 
   for (const i of arr) {
     untar({

diff --git a/tap-snapshots/test-index.js-TAP.test.js b/tap-snapshots/test-index.js-TAP.test.js
@@ -5,50 +5,6 @@
  * Make sure to inspect the output below.  Do not ignore changes!
  */
 'use strict'
-exports[`test/index.js TAP compare current dir with a given spec > should output diff against cwd files 1`] = `
-diff --git a/index.js b/index.js
-index v1.0.1..v1.0.0 100644
---- a/index.js
-+++ b/index.js
-@@ -1,3 +1,2 @@
--const bar = "bar"
- module.exports =
--  bar
-+  "foo"
-diff --git a/package.json b/package.json
-index v1.0.1..v1.0.0 100644
---- a/package.json
-+++ b/package.json
-@@ -1,4 +1,4 @@
- {
-   "name": "a",
--  "version": "1.0.1"
-+  "version": "1.0.0"
- }
-`
-
-exports[`test/index.js TAP compare current dir with a given spec no opts > should output diff against cwd files 1`] = `
-diff --git a/index.js b/index.js
-index v1.0.1..v1.0.0 100644
---- a/index.js
-+++ b/index.js
-@@ -1,3 +1,2 @@
--const bar = "bar"
- module.exports =
--  bar
-+  "foo"
-diff --git a/package.json b/package.json
-index v1.0.1..v1.0.0 100644
---- a/package.json
-+++ b/package.json
-@@ -1,4 +1,4 @@
- {
-   "name": "a",
--  "version": "1.0.1"
-+  "version": "1.0.0"
- }
-`
-
 exports[`test/index.js TAP compare two diff specs > should output expected diff 1`] = `
 diff --git a/index.js b/index.js
 index v1.0.0..v2.0.0 100644

diff --git a/test.js b/test.js
diff --git a/test/format-diff.js b/test/format-diff.js
@@ -283,9 +283,7 @@ t.test('using --name-only option', t => {
       refs,
       versions,
       opts: {
-        diffOpts: {
-          nameOnly: true,
-        },
+        diffNameOnly: true,
       },
     }),
     'should output expected diff result'
@@ -354,12 +352,10 @@ t.test('diff options', t => {
       refs,
       versions,
       opts: {
-        diffOpts: {
-          context: 1,
-          ignoreWhitespace: true,
-          srcPrefix: 'before/',
-          dstPrefix: 'after/',
-        },
+        diffContext: 1,
+        diffIgnoreWhitespace: true,
+        diffSrcPrefix: 'before/',
+        diffDstPrefix: 'after/',
       },
     }),
     'should output expected diff result'
@@ -392,9 +388,7 @@ t.test('noPrefix', t => {
       refs,
       versions,
       opts: {
-        diffOpts: {
-          noPrefix: true,
-        },
+        diffNoPrefix: true,
       },
     }),
     'should output result with no prefixes'

diff --git a/test/index.js b/test/index.js
@@ -36,61 +36,22 @@ t.test('compare two diff specs', async t => {
   const a = `file:${resolve(path, 'a1')}`
   const b = `file:${resolve(path, 'a2')}`
 
-  t.resolveMatchSnapshot(diff({ a, b }, {}), 'should output expected diff')
+  t.resolveMatchSnapshot(diff([a, b], {}), 'should output expected diff')
 })
 
-t.test('compare current dir with a given spec', async t => {
-  const path = t.testdir({
-    cwd: {
-      'package.json': json({
-        name: 'a',
-        version: '1.0.0',
-      }),
-      'index.js': 'module.exports =\n  "foo"\n',
-    },
-    diff: {
-      'package.json': json({
-        name: 'a',
-        version: '1.0.1',
-      }),
-      'index.js': 'const bar = "bar"\nmodule.exports =\n  bar\n',
-    },
-  })
-
-  const cwd = resolve(path, 'cwd')
-  const a = `file:${resolve(path, 'diff')}`
-
-  t.resolveMatchSnapshot(
-    diff({ a }, { prefix: cwd }), 'should output diff against cwd files')
+t.test('using single arg', async t => {
+  await t.rejects(
+    diff(['abbrev@1.0.3']),
+    /libnpmdiff needs two arguments to compare/,
+    'should throw EDIFFARGS error'
+  )
 })
 
-t.test('compare current dir with a given spec no opts', async t => {
-  const path = t.testdir({
-    cwd: {
-      'package.json': json({
-        name: 'a',
-        version: '1.0.0',
-      }),
-      'index.js': 'module.exports =\n  "foo"\n',
-    },
-    diff: {
-      'package.json': json({
-        name: 'a',
-        version: '1.0.1',
-      }),
-      'index.js': 'const bar = "bar"\nmodule.exports =\n  bar\n',
-    },
-  })
-
-  const cwd = resolve(path, 'cwd')
-  const a = `file:${resolve(path, 'diff')}`
-
-  const _cwd = process.cwd()
-  process.chdir(cwd)
-  t.teardown(() => {
-    process.chdir(_cwd)
-  })
-
-  t.resolveMatchSnapshot(
-    diff({ a }), 'should output diff against cwd files')
+t.test('too many args', async t => {
+  const args = ['abbrev@1.0.3', 'abbrev@1.0.4', 'abbrev@1.0.5']
+  await t.rejects(
+    diff(args),
+    /libnpmdiff needs two arguments to compare/,
+    'should output diff against cwd files'
+  )
 })
diff --git a/test/should-print-patch.js b/test/should-print-patch.js
@@ -19,7 +19,7 @@ t.test('invalid filenames', t => {
 
 t.test('using --text/-a option', t => {
   const opts = {
-    diffOpts: { text: true },
+    diffText: true,
   }
   t.ok(shouldPrintPatch('foo.exe', opts))
   t.ok(shouldPrintPatch('./foo.jpg', opts))

diff --git a/test/untar.js b/test/untar.js
@@ -57,13 +57,11 @@ t.test('filter files', async t => {
     item,
     prefix: 'a/',
   }, {
-    diffOpts: {
-      files: [
-        './LICENSE',
-        'missing-file',
-        'README.md',
-      ],
-    },
+    diffFiles: [
+      './LICENSE',
+      'missing-file',
+      'README.md',
+    ],
   })
 
   t.matchSnapshot([...files].join('\n'), 'should return list of filenames')
@@ -106,13 +104,11 @@ t.test('filter files using glob expressions', async t => {
     item,
     prefix: 'a/',
   }, {
-    diffOpts: {
-      files: [
-        './lib/**',
-        '*-lock.json',
-        'test\\*', // windows-style sep should be normalized
-      ],
-    },
+    diffFiles: [
+      './lib/**',
+      '*-lock.json',
+      'test\\*', // windows-style sep should be normalized
+    ],
   })
 
   t.matchSnapshot([...files].join('\n'), 'should return list of filenames')
@@ -133,11 +129,9 @@ t.test('filter out all files', async t => {
     item,
     prefix: 'a/',
   }, {
-    diffOpts: {
-      files: [
-        'non-matching-pattern',
-      ],
-    },
+    diffFiles: [
+      'non-matching-pattern',
+    ],
   })
 
   t.equal(files.size, 0, 'should have no files')