From 7c73314122b6dc53378051895d8ab3984c3d6d99 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 15 Jan 2025 09:25:40 -0800 Subject: [PATCH 1/2] chore: scope test fixture package names --- tap-snapshots/test/index.js.test.cjs | 125 +----------------- .../all-fields-populated/package.json | 4 +- test/index.js | 12 +- 3 files changed, 11 insertions(+), 130 deletions(-) diff --git a/tap-snapshots/test/index.js.test.cjs b/tap-snapshots/test/index.js.test.cjs index ca55392..36b0301 100644 --- a/tap-snapshots/test/index.js.test.cjs +++ b/tap-snapshots/test/index.js.test.cjs @@ -27,137 +27,18 @@ exports[`test/index.js TAP load create:true existing parseable package.json > pa ` exports[`test/index.js TAP load custom formatting > should save back custom format to package.json 1`] = ` -{"name":"foo","version":"1.0.1","description":"Lorem ipsum dolor"} +{"name":"@npmcli/test","version":"1.0.1","description":"Lorem ipsum dolor"} ` exports[`test/index.js TAP load read, update content and write > should properly save content to a package.json 1`] = ` { - "name": "foo", + "name": "@npmcli/test", "version": "1.0.1", "description": "Lorem ipsum dolor" } ` -exports[`test/index.js TAP load sorts on save > should properly save content to a package.json 1`] = ` -{ - "name": "foo", - "version": "1.0.0", - "description": "A sample package", - "keywords": [ - "sample", - "package" - ], - "homepage": "https://example.com", - "bugs": { - "url": "https://example.com/bugs", - "email": "bugs@example.com" - }, - "repository": { - "type": "git", - "url": "https://example.com/repo.git" - }, - "funding": "https://example.com/funding", - "license": "MIT", - "author": "Author Name ", - "maintainers": [ - "Maintainer One ", - "Maintainer Two " - ], - "contributors": [ - "Contributor One ", - "Contributor Two " - ], - "type": "module", - "imports": { - "#dep": "./src/dep.js" - }, - "exports": { - ".": "./src/index.js" - }, - "main": "index.js", - "browser": "browser.js", - "types": "index.d.ts", - "bin": { - "my-cli": "./bin/cli.js" - }, - "man": [ - "./man/doc.1" - ], - "directories": { - "lib": "lib", - "bin": "bin", - "man": "man" - }, - "files": [ - "lib/**/*.js", - "bin/**/*.js" - ], - "workspaces": [ - "packages/*" - ], - "scripts": { - "start": "node index.js", - "test": "tap test/*.js" - }, - "config": { - "port": "8080" - }, - "dependencies": { - "some-dependency": "^1.0.0" - }, - "devDependencies": { - "some-dev-dependency": "^1.0.0" - }, - "peerDependencies": { - "some-peer-dependency": "^1.0.0" - }, - "peerDependenciesMeta": { - "some-peer-dependency": { - "optional": true - } - }, - "optionalDependencies": { - "some-optional-dependency": "^1.0.0" - }, - "bundledDependencies": [ - "some-bundled-dependency" - ], - "bundleDependencies": [ - "some-bundled-dependency" - ], - "engines": { - "node": ">=14.0.0" - }, - "os": [ - "darwin", - "linux" - ], - "cpu": [ - "x64", - "arm64" - ], - "publishConfig": { - "registry": "https://registry.example.com" - }, - "devEngines": { - "node": ">=14.0.0" - }, - "licenses": [ - { - "type": "MIT", - "url": "https://opensource.org/licenses/MIT" - } - ], - "overrides": { - "some-dependency": { - "some-sub-dependency": "1.0.0" - } - } -} - -` - exports[`test/index.js TAP load update long package.json > should only update the defined property 1`] = ` { "version": "7.18.1", @@ -649,7 +530,7 @@ exports[`test/index.js TAP load update long package.json > should properly write exports[`test/index.js TAP read package > must match snapshot 1`] = ` Object { - "name": "foo", + "name": "@npmcli/test", "version": "1.0.0", } ` diff --git a/test/fixtures/all-fields-populated/package.json b/test/fixtures/all-fields-populated/package.json index 508db47..6b27829 100644 --- a/test/fixtures/all-fields-populated/package.json +++ b/test/fixtures/all-fields-populated/package.json @@ -1,5 +1,5 @@ { - "name": "foo", + "name": "@npmcli/test", "version": "1.0.0", "private": true, "description": "A sample package", @@ -93,4 +93,4 @@ "some-sub-dependency": "1.0.0" } } -} \ No newline at end of file +} diff --git a/test/index.js b/test/index.js index baf0564..00c8557 100644 --- a/test/index.js +++ b/test/index.js @@ -22,7 +22,7 @@ t.test('load', t => { t.test('read a valid package.json', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ - name: 'foo', + name: '@npmcli/test', version: '1.0.0', }), }) @@ -30,14 +30,14 @@ t.test('load', t => { const pj = await PackageJson.load(path) t.same( pj.content, - { name: 'foo', version: '1.0.0' }, + { name: '@npmcli/test', version: '1.0.0' }, 'should return content for a valid package.json' ) }) t.test('read, update content and write', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ - name: 'foo', + name: '@npmcli/test', version: '1.0.0', }, null, 8), }) @@ -65,7 +65,7 @@ t.test('load', t => { ) }) t.test('do not overwite unchanged file on EOF line added/removed', async t => { - const originalPackageJsonContent = '{\n "name": "foo"\n}' + const originalPackageJsonContent = '{\n "name": "@npmcli/test"\n}' const path = t.testdir({ 'package.json': originalPackageJsonContent, }) @@ -132,7 +132,7 @@ t.test('load', t => { t.test('custom formatting', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ - name: 'foo', + name: '@npmcli/test', version: '1.0.0', }, null, 0), }) @@ -243,7 +243,7 @@ t.test('read package', async t => { const { readPackage } = require('../lib/read-package') const path = t.testdir({ 'package.json': JSON.stringify({ - name: 'foo', + name: '@npmcli/test', version: '1.0.0', }), }) From 841ee3b673744184eb574cd25f13d2e87b919b26 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 15 Jan 2025 09:30:23 -0800 Subject: [PATCH 2/2] fix: save when reverting content When a package.json is changed, saved, and then reverted back to what it was before, save was not running because this module was not updating its local copy of what the contents of the file was. This has been fixed. --- lib/index.js | 4 +++- test/index.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/index.js b/lib/index.js index 23f326d..828b899 100644 --- a/lib/index.js +++ b/lib/index.js @@ -252,7 +252,9 @@ class PackageJson { .replace(/\n/g, eol) if (fileContent.trim() !== this.#readFileContent.trim()) { - return await writeFile(this.filename, fileContent) + const written = await writeFile(this.filename, fileContent) + this.#readFileContent = fileContent + return written } } diff --git a/test/index.js b/test/index.js index 00c8557..d724286 100644 --- a/test/index.js +++ b/test/index.js @@ -327,3 +327,31 @@ t.test('empty props at bottom', async t => { JSON.parse(fs.readFileSync(resolve(path, 'package.json'), 'utf8')) ) }) + +t.test('reversion can still save', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: '@npmcli/test', + version: '1.0.0', + }), + }) + const pkgJson = await PackageJson.load(path) + pkgJson.update({ version: '2.0.0' }) + await pkgJson.save() + t.strictSame( + { + name: '@npmcli/test', + version: '2.0.0', + }, + JSON.parse(fs.readFileSync(resolve(path, 'package.json'), 'utf8')) + ) + pkgJson.update({ version: '1.0.0' }) + await pkgJson.save() + t.strictSame( + { + name: '@npmcli/test', + version: '1.0.0', + }, + JSON.parse(fs.readFileSync(resolve(path, 'package.json'), 'utf8')) + ) +})