fix: move path parameter to load

wraithgar · wraithgar · commit c3d11c2a91d7 · 2023-06-21T13:18:15.000-07:00
BREAKING CHANGE: the `path` parameter is now tied to `load` and not the
class constructor.
diff --git a/README.md b/README.md
@@ -52,28 +52,24 @@ await pkgJson.save()
 
 ## API:
 
-### `constructor(path)`
+### `constructor()`
 
-Creates a new instance of `PackageJson`.
-
-- `path`: `String` that points to the folder from where to read the
-`package.json` from
+Creates a new empty instance of `PackageJson`.
 
 ---
 
-### `async PackageJson.load()`
+### `async PackageJson.load(path)`
 
-Loads the `package.json` at location determined in the `path` option of
-the constructor.
+Loads the `package.json` at the given path.
 
 ### Example:
 
 Loads contents of the `package.json` file located at `./`:
 
 ```js
 const PackageJson = require('@npmcli/package-json')
-const pkgJson = new PackageJson('./')
-await pkgJson.load()
+const pkgJson = new PackageJson()
+await pkgJson.load('./')
 ```
 
 Throws an error in case the `package.json` file is missing or has invalid
@@ -102,29 +98,27 @@ const pkgJson = await PackageJson.load('./')
 
 ### `async PackageJson.normalize()`
 
-Like `load` but intended for reading 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`
 
 ---
 
 ### **static** `async PackageJson.normalize(path)`
 
-Convenience static method like `load` but for calling `normalize`
-
----
+Convenience static that calls `load` before calling `normalize`
 
 ---
 
 ### `async PackageJson.prepare()`
 
-Like `load` but intended for reading package.json files before publish.
+Like `normalize` but intended for preparing package.json files for publish.
 
 ---
 
 ### **static** `async PackageJson.prepare(path)`
 
-Convenience static method like `load` but for calling `prepare`
+Convenience static that calls `load` before calling `prepare`
 
 ---
 
@@ -137,7 +131,7 @@ Updates the contents of the `package.json` with the `content` provided.
 
 Special properties like `dependencies`, `devDependencies`,
 `optionalDependencies`, `peerDependencies` will have special logic to handle
-the update of these options, such as deduplications.
+the update of these options, such as sorting and deduplication.
 
 ### Example:
 
@@ -200,22 +194,8 @@ pkgJson.content
 
 ### `async PackageJson.save()`
 
-Saves the current `content` to the same location used when initializing
-this instance.
-
-<br />
-
-## Related
-
-When you make a living out of reading and writing `package.json` files, you end
-up with quite the amount of packages dedicated to it, the **npm cli** also
-uses:
-
-- [read-package-json-fast](https://github.com/npm/read-package-json-fast) reads
-and normalizes `package.json` files the way the **npm cli** expects it.
-- [read-package-json](https://github.com/npm/read-package-json) reads and
-normalizes more info from your `package.json` file. Used by `npm@6` and in
-`npm@7` for publishing.
+Saves the current `content` to the same location used when calling
+`load()`.
 
 ## LICENSE
 
diff --git a/lib/index.js b/lib/index.js
@@ -52,37 +52,37 @@ class PackageJson {
     'binRefs',
   ])
 
-  // default behavior, just loads and parses
+  // default behavior, loads a package.json at given path and JSON parses
   static async load (path) {
-    return await new PackageJson(path).load()
+    const p = new PackageJson()
+    return p.load(path)
   }
 
   // read-package-json compatible behavior
   static async prepare (path, opts) {
-    return await new PackageJson(path).prepare(opts)
+    const p = new PackageJson()
+    await p.load(path, true)
+    return p.prepare(opts)
   }
 
   // read-package-json-fast compatible behavior
   static async normalize (path, opts) {
-    return await new PackageJson(path).normalize(opts)
+    const p = new PackageJson()
+    await p.load(path)
+    return p.normalize(opts)
   }
 
-  #filename
   #path
-  #manifest = {}
+  #manifest
   #readFileContent = ''
-  #fromIndex = false
+  #canSave = true
 
-  constructor (path) {
+  // Load content from given path
+  async load (path, parseIndex) {
     this.#path = path
-    this.#filename = resolve(path, 'package.json')
-  }
-
-  async load (parseIndex) {
     let parseErr
     try {
-      this.#readFileContent =
-        await readFile(this.#filename, 'utf8')
+      this.#readFileContent = await readFile(this.filename, 'utf8')
     } catch (err) {
       err.message = `Could not read package.json: ${err}`
       if (!parseIndex) {
@@ -92,31 +92,58 @@ class PackageJson {
     }
 
     if (parseErr) {
-      const indexFile = resolve(this.#path, 'index.js')
+      const indexFile = resolve(this.path, 'index.js')
       let indexFileContent
       try {
         indexFileContent = await readFile(indexFile, 'utf8')
       } catch (err) {
         throw parseErr
       }
       try {
-        this.#manifest = fromComment(indexFileContent)
+        this.fromComment(indexFileContent)
       } catch (err) {
         throw parseErr
       }
-      this.#fromIndex = true
+      // This wasn't a package.json so prevent saving
+      this.#canSave = false
       return this
     }
 
+    return this.fromJSON(this.#readFileContent)
+  }
+
+  // Load data from a JSON string/buffer
+  fromJSON (data) {
     try {
-      this.#manifest = parseJSON(this.#readFileContent)
+      this.#manifest = parseJSON(data)
     } catch (err) {
       err.message = `Invalid package.json: ${err}`
       throw err
     }
     return this
   }
 
+  // Load data from a comment
+  // /**package { "name": "foo", "version": "1.2.3", ... } **/
+  fromComment (data) {
+    data = data.split(/^\/\*\*package(?:\s|$)/m)
+
+    if (data.length < 2) {
+      throw new Error('File has no package in comments')
+    }
+    data = data[1]
+    data = data.split(/\*\*\/$/m)
+
+    if (data.length < 2) {
+      throw new Error('File has no package in comments')
+    }
+    data = data[0]
+    data = data.replace(/^\s*\*/mg, '')
+
+    this.#manifest = parseJSON(data)
+    return this
+  }
+
   get content () {
     return this.#manifest
   }
@@ -125,58 +152,59 @@ class PackageJson {
     return this.#path
   }
 
-  update (content) {
-    // validates both current manifest and content param
-    const invalidContent =
-      typeof this.#manifest !== 'object'
-        || typeof content !== 'object'
-    if (invalidContent) {
-      throw Object.assign(
-        new Error(`Can't update invalid package.json data`),
-        { code: 'EPACKAGEJSONUPDATE' }
-      )
+  get filename () {
+    if (this.path) {
+      return resolve(this.path, 'package.json')
     }
+    return undefined
+  }
 
+  update (content) {
+    if (!this.content) {
+      throw new Error('Can not update without existing data')
+    }
     for (const step of knownSteps) {
-      this.#manifest = step({ content, originalContent: this.#manifest })
+      this.#manifest = step({ content, originalContent: this.content })
     }
 
     // unknown properties will just be overwitten
     for (const [key, value] of Object.entries(content)) {
       if (!knownKeys.has(key)) {
-        this.#manifest[key] = value
+        this.content[key] = value
       }
     }
 
     return this
   }
 
   async save () {
-    if (this.#fromIndex) {
+    if (!this.#canSave) {
       throw new Error('No package.json to save to')
     }
     const {
       [Symbol.for('indent')]: indent,
       [Symbol.for('newline')]: newline,
-    } = this.#manifest
+    } = this.content
 
-    const format = indent === undefined ? '  ' : indent
-    const eol = newline === undefined ? '\n' : newline
+    // These can only be undefined if we didn't parse JSON, which we currently don't support.
+    // const format = indent === undefined ? '  ' : indent
+    // const eol = newline === undefined ? '\n' : newline
+    const format = indent
+    const eol = newline
     const fileContent = `${
-      JSON.stringify(this.#manifest, null, format)
+      JSON.stringify(this.content, null, format)
     }\n`
       .replace(/\n/g, eol)
 
     if (fileContent.trim() !== this.#readFileContent.trim()) {
-      return await writeFile(this.#filename, fileContent)
+      return await writeFile(this.filename, fileContent)
     }
   }
 
   async normalize (opts = {}) {
     if (!opts.steps) {
       opts.steps = this.constructor.normalizeSteps
     }
-    await this.load()
     await normalize(this, opts)
     return this
   }
@@ -185,29 +213,9 @@ class PackageJson {
     if (!opts.steps) {
       opts.steps = this.constructor.prepareSteps
     }
-    await this.load(true)
     await normalize(this, opts)
     return this
   }
 }
 
-// /**package { "name": "foo", "version": "1.2.3", ... } **/
-function fromComment (data) {
-  data = data.split(/^\/\*\*package(?:\s|$)/m)
-
-  if (data.length < 2) {
-    throw new Error('File has no package in comments')
-  }
-  data = data[1]
-  data = data.split(/\*\*\/$/m)
-
-  if (data.length < 2) {
-    throw new Error('File has no package in comments')
-  }
-  data = data[0]
-  data = data.replace(/^\s*\*/mg, '')
-
-  return parseJSON(data)
-}
-
 module.exports = PackageJson
diff --git a/lib/normalize.js b/lib/normalize.js
@@ -7,6 +7,9 @@ const log = require('proc-log')
 const git = require('@npmcli/git')
 
 const normalize = async (pkg, { strict, steps, root }) => {
+  if (!pkg.content) {
+    throw new Error('Can not normalize without existing data')
+  }
   const data = pkg.content
   const scripts = data.scripts || {}
   const pkgId = `${data.name ?? ''}@${data.version ?? ''}`
diff --git a/test/index.js b/test/index.js
