From 7e8b82472b60a047be6603746c82536f36c1d8d6 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 17 Feb 2021 12:21:48 -0800 Subject: [PATCH] Do not write package.json if nothing changed Need a similar fix for Shrinkwrap class. Fix: https://github.com/npm/arborist/issues/232 PR-URL: https://github.com/npm/arborist/pull/234 Credit: @isaacs Close: #234 Reviewed-by: @nlf --- lib/update-root-package-json.js | 16 ++++++-- test/update-root-package-json.js | 70 ++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/lib/update-root-package-json.js b/lib/update-root-package-json.js index 735ebd10a..56965e26d 100644 --- a/lib/update-root-package-json.js +++ b/lib/update-root-package-json.js @@ -15,11 +15,18 @@ const depTypes = new Set([ 'peerDependencies', ]) +const parseJsonSafe = json => { + try { + return parseJSON(json) + } catch (er) { + return null + } +} + const updateRootPackageJson = async tree => { const filename = resolve(tree.path, 'package.json') - const originalContent = await readFile(filename, 'utf8') - .then(data => parseJSON(data)) - .catch(() => null) + const originalJson = await readFile(filename, 'utf8').catch(() => null) + const originalContent = parseJsonSafe(originalJson) const depsData = orderDeps({ ...tree.package, @@ -52,7 +59,8 @@ const updateRootPackageJson = async tree => { const content = (JSON.stringify(packageJsonContent, null, format) + '\n') .replace(/\n/g, eol) - return writeFile(filename, content) + if (content !== originalJson) + return writeFile(filename, content) } module.exports = updateRootPackageJson diff --git a/test/update-root-package-json.js b/test/update-root-package-json.js index 7f88eeda5..be18d0c8a 100644 --- a/test/update-root-package-json.js +++ b/test/update-root-package-json.js @@ -1,4 +1,5 @@ const {resolve} = require('path') +const {statSync} = require('fs') const t = require('tap') @@ -29,6 +30,33 @@ t.test('missing package.json', async t => { ) }) +t.test('invalid package.json', async t => { + const path = t.testdir({ + 'package.json': 'this! is! not! json!', + }) + await updateRootPackageJson({ + path: path, + package: { + name: 'invalid-package-json-test', + version: '1.0.0', + dependencies: { + abbrev: '^1.0.0', + }, + }, + }) + t.match( + require(resolve(path, 'package.json')), + { + name: 'invalid-package-json-test', + version: '1.0.0', + dependencies: { + abbrev: '^1.0.0', + }, + }, + 'should write new package.json with tree data' + ) +}) + t.test('existing package.json', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ @@ -62,6 +90,48 @@ t.test('existing package.json', async t => { ) }) +t.test('unchanged package.json', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'existing-package-json-test', + version: '1.0.0', + bin: './file.js', + funding: 'http://example.com', + dependencies: { + abbrev: '^1.0.0', + }, + }, null, 2) + '\n', + }) + const { mtime } = statSync(path + '/package.json') + await updateRootPackageJson({ + path: path, + package: { + name: 'existing-package-json-test', + version: '1.0.0', + bin: './file.js', + funding: 'http://example.com', + dependencies: { + abbrev: '^1.0.0', + }, + }, + }) + t.match( + require(resolve(path, 'package.json')), + { + name: 'existing-package-json-test', + version: '1.0.0', + bin: './file.js', + funding: 'http://example.com', + dependencies: { + abbrev: '^1.0.0', + }, + }, + 'should write new package.json with tree data' + ) + const { mtime: newMtime } = statSync(path + '/package.json') + t.equal(newMtime.toISOString(), mtime.toISOString(), 'mtime not changed') +}) + t.test('existing package.json with optionalDependencies', async t => { const path = t.testdir({ 'package.json': JSON.stringify({