feat: add changes array to normalize functions (#41)

npm · Jun 26, 2023 · 7ddb1d1 · 7ddb1d1
1 parent 4775bf3
commit 7ddb1d1
Show file tree

Hide file tree

Showing 4 changed files with 237 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -70,9 +70,7 @@ it will be overwritten.
 Loads a `package.json` at the given path.
 
 - `opts`: `Object` can contain:
-  - `create`: `Boolean` if true, a new package.json will be created if
-      one does not already exist. Will not clobber ane existing
-      package.json that can not be parsed.
+  - `create`: `Boolean` if true, a new package.json will be created if one does not already exist. Will not clobber ane existing package.json that can not be parsed.
 
 ### Example:
 
@@ -84,18 +82,15 @@ const pkgJson = new PackageJson()
 await pkgJson.load('./')
 ```
 
-Throws an error in case a `package.json` file is missing or has invalid
-contents.
+Throws an error in case a `package.json` file is missing or has invalid contents.
 
 ---
 
 ### **static** `async PackageJson.load(path)`
 
-Convenience static method that returns a new instance and loads the contents of
-a `package.json` file from that location.
+Convenience static method that returns a new instance and loads the contents of a `package.json` file from that location.
 
-- `path`: `String` that points to the folder from where to read the
-`package.json` from
+- `path`: `String` that points to the folder from where to read the `package.json` from
 
 ### Example:
 
@@ -110,16 +105,28 @@ const pkgJson = await PackageJson.load('./')
 
 ### `async PackageJson.normalize()`
 
-Intended for normalizing package.json files in a node_modules tree.
-Some light normalization is done to ensure that it is ready for use in
-`@npmcli/arborist`
+Intended for normalizing package.json files in a node_modules tree.  Some light normalization is done to ensure that it is ready for use in `@npmcli/arborist`
+
+- `path`: `String` that points to the folder from where to read the `package.json` from
+- `opts`: `Object` can contain:
+  - `strict`: `Boolean` enables optional strict mode when applying the `normalizeData` step
+  - `steps`: `Array` optional normalization steps that will be applied to the `package.json` file, replacing the default steps
+  - `root`: `Path` optional git root to provide when applying the `gitHead` step
+  - `changes`: `Array` if provided, a message about each change that was made to the packument will be added to this array
 
 ---
 
-### **static** `async PackageJson.normalize(path)`
+### **static** `async PackageJson.normalize(path, opts = {})`
 
 Convenience static that calls `load` before calling `normalize`
 
+- `path`: `String` that points to the folder from where to read the `package.json` from
+- `opts`: `Object` can contain:
+  - `strict`: `Boolean` enables optional strict mode when applying the `normalizeData` step
+  - `steps`: `Array` optional normalization steps that will be applied to the `package.json` file, replacing the default steps
+  - `root`: `Path` optional git root to provide when applying the `gitHead` step
+  - `changes`: `Array` if provided, a message about each change that was made to the packument will be added to this array
+
 ---
 
 ### `async PackageJson.prepare()`
@@ -128,10 +135,17 @@ Like `normalize` but intended for preparing package.json files for publish.
 
 ---
 
-### **static** `async PackageJson.prepare(path)`
+### **static** `async PackageJson.prepare(path, opts = {})`
 
 Convenience static that calls `load` before calling `prepare`
 
+- `path`: `String` that points to the folder from where to read the `package.json` from
+- `opts`: `Object` can contain:
+  - `strict`: `Boolean` enables optional strict mode when applying the `normalizeData` step
+  - `steps`: `Array` optional normalization steps that will be applied to the `package.json` file, replacing the default steps
+  - `root`: `Path` optional git root to provide when applying the `gitHead` step
+  - `changes`: `Array` if provided, a message about each change that was made to the packument will be added to this array
+
 ---
 
 ### `PackageJson.update(content)`

diff --git a/lib/normalize.js b/lib/normalize.js
@@ -6,7 +6,10 @@ const path = require('path')
 const log = require('proc-log')
 const git = require('@npmcli/git')
 
-const normalize = async (pkg, { strict, steps, root }) => {
+// We don't want the `changes` array in here by default because this is a hot
+// path for parsing packuments during install.  So the calling method passes it
+// in if it wants to track changes.
+const normalize = async (pkg, { strict, steps, root, changes }) => {
   if (!pkg.content) {
     throw new Error('Can not normalize without content')
   }
@@ -18,6 +21,7 @@ const normalize = async (pkg, { strict, steps, root }) => {
   if (steps.includes('_attributes')) {
     for (const key in data) {
       if (key.startsWith('_')) {
+        changes?.push(`"${key}" was removed`)
         delete pkg.content[key]
       }
     }
@@ -26,6 +30,7 @@ const normalize = async (pkg, { strict, steps, root }) => {
   // build the "_id" attribute
   if (steps.includes('_id')) {
     if (data.name && data.version) {
+      changes?.push(`"_id" was set to ${pkgId}`)
       data._id = pkgId
     }
   }
@@ -35,20 +40,25 @@ const normalize = async (pkg, { strict, steps, root }) => {
     if (data.bundleDependencies === undefined && data.bundledDependencies !== undefined) {
       data.bundleDependencies = data.bundledDependencies
     }
+    changes?.push(`Deleted incorrect "bundledDependencies"`)
     delete data.bundledDependencies
   }
   // expand "bundleDependencies: true or translate from object"
   if (steps.includes('bundleDependencies')) {
     const bd = data.bundleDependencies
     if (bd === false && !steps.includes('bundleDependenciesDeleteFalse')) {
+      changes?.push(`"bundleDepdencies" was changed from "false" to "[]"`)
       data.bundleDependencies = []
     } else if (bd === true) {
+      changes?.push(`"bundleDepdencies" was auto-populated from "dependencies"`)
       data.bundleDependencies = Object.keys(data.dependencies || {})
     } else if (bd && typeof bd === 'object') {
       if (!Array.isArray(bd)) {
+        changes?.push(`"bundleDependencies" was changed from an object to an array`)
         data.bundleDependencies = Object.keys(bd)
       }
     } else {
+      changes?.push(`"bundleDependencies" was removed`)
       delete data.bundleDependencies
     }
   }
@@ -61,9 +71,11 @@ const normalize = async (pkg, { strict, steps, root }) => {
     if (data.dependencies &&
       data.optionalDependencies && typeof data.optionalDependencies === 'object') {
       for (const name in data.optionalDependencies) {
+        changes?.push(`optionalDependencies entry "${name}" was removed`)
         delete data.dependencies[name]
       }
       if (!Object.keys(data.dependencies).length) {
+        changes?.push(`empty "optionalDependencies" was removed`)
         delete data.dependencies
       }
     }
@@ -77,6 +89,8 @@ const normalize = async (pkg, { strict, steps, root }) => {
         scripts.install = 'node-gyp rebuild'
         data.scripts = scripts
         data.gypfile = true
+        changes?.push(`"scripts.install" was set to "node-gyp rebuild"`)
+        changes?.push(`"gypfile" was set to "true"`)
       }
     }
   }
@@ -87,6 +101,7 @@ const normalize = async (pkg, { strict, steps, root }) => {
       await fs.access(path.join(pkg.path, 'server.js'))
       scripts.start = 'node server.js'
       data.scripts = scripts
+      changes?.push('"scripts.start" was set to "node server.js"')
     } catch {
       // do nothing
     }
@@ -99,18 +114,22 @@ const normalize = async (pkg, { strict, steps, root }) => {
       for (const name in data.scripts) {
         if (typeof data.scripts[name] !== 'string') {
           delete data.scripts[name]
+          changes?.push(`invalid scripts entry "${name}" was removed`)
         } else if (steps.includes('scriptpath')) {
           data.scripts[name] = data.scripts[name].replace(spre, '')
+          changes?.push(`scripts entry "${name}" was fixed to remove node_modules/.bin reference`)
         }
       }
     } else {
+      changes?.push(`removed invalid "scripts"`)
       delete data.scripts
     }
   }
 
   if (steps.includes('funding')) {
     if (data.funding && typeof data.funding === 'string') {
       data.funding = { url: data.funding }
+      changes?.push(`"funding" was changed to an object with a url attribute`)
     }
   }
 
@@ -122,6 +141,7 @@ const normalize = async (pkg, { strict, steps, root }) => {
         .map(line => line.replace(/^\s*#.*$/, '').trim())
         .filter(line => line)
       data.contributors = authors
+      changes.push('"contributors" was auto-populated with the contents of the "AUTHORS" file')
     } catch {
       // do nothing
     }
@@ -148,6 +168,8 @@ const normalize = async (pkg, { strict, steps, root }) => {
       const readmeData = await fs.readFile(path.join(pkg.path, readmeFile), 'utf8')
       data.readme = readmeData
       data.readmeFilename = readmeFile
+      changes?.push(`"readme" was set to the contents of ${readmeFile}`)
+      changes?.push(`"readmeFilename" was set to ${readmeFile}`)
     }
   }
 

diff --git a/tap-snapshots/test/normalize.js.test.cjs b/tap-snapshots/test/normalize.js.test.cjs
@@ -0,0 +1,176 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies change name if bundleDependencies is not present > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies dont array-ify if its an array already > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies handle bundleDependencies object > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was changed from an object to an array",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies handle bundleDependencies: false > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDepdencies\\" was changed from \\"false\\" to \\"[]\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies handle bundleDependencies: true > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDepdencies\\" was auto-populated from \\"dependencies\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies handle bundleDependencies: true with no deps > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDepdencies\\" was auto-populated from \\"dependencies\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up bundleDependencies handle bundledDependencies: true > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDepdencies\\" was auto-populated from \\"dependencies\\"",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up scripts delete non-object scripts > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes clean up scripts delete non-string script targets > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "invalid scripts entry \\"bar\\" was removed",
+  "invalid scripts entry \\"baz\\" was removed",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins delete string bin when no name > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins handle string when a name is set > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins remove non-object bin > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins remove non-string bin values > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes convert funding string to object > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+  "\\"funding\\" was changed to an object with a url attribute",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes dedupe optional deps out of regular deps choose optional deps in conflict, leaving populated dependencies > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "optionalDependencies entry \\"whowins\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes dedupe optional deps out of regular deps choose optional deps in conflict, removing empty dependencies > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "optionalDependencies entry \\"whowins\\" was removed",
+  "empty \\"optionalDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes dedupe optional deps out of regular deps do not create regular deps if only optional specified > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes normalize bin > must match snapshot 1`] = `
+Array [
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes set _id if name and version set > must match snapshot 1`] = `
+Array [
+  "\\"_id\\" was set to a@1.2.3",
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes skipping steps > must match snapshot 1`] = `
+Array []
+`
+
+exports[`test/normalize.js TAP @npmcli/package-json - with changes strip _fields > must match snapshot 1`] = `
+Array [
+  "\\"_lodash\\" was removed",
+  "\\"_id\\" was set to underscore@1.2.3",
+  "Deleted incorrect \\"bundledDependencies\\"",
+  "\\"bundleDependencies\\" was removed",
+  "removed invalid \\"scripts\\"",
+]
+`