From 510f0ecbc9970ed8c8993107cc03cf27b7b996dc Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Tue, 18 Jan 2022 11:24:53 -0800 Subject: [PATCH] fix(arborist): ensure indentation is preserved (#4218) It turns out that `new Arborist().buildIdealTree().meta.toString()` does not take into account the indentation in the package.json (tabs, in my case) the way `npm install --package-lock-only` does. This fixes that. Also included a bonus commit that removes redundant Promise stuff inside an `async function`. --- .../arborist/lib/arborist/build-ideal-tree.js | 6 ++-- workspaces/arborist/lib/shrinkwrap.js | 22 +++++++------ .../arborist/build-ideal-tree.js.test.cjs | 31 +++++++++++++++++++ .../test/arborist/build-ideal-tree.js | 13 ++++++++ 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 899d92ca937cc..dffcd546b8677 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -176,7 +176,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { // public method async buildIdealTree (options = {}) { if (this.idealTree) { - return Promise.resolve(this.idealTree) + return this.idealTree } // allow the user to set reify options on the ctor as well. @@ -194,8 +194,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { process.emit('time', 'idealTree') if (!options.add && !options.rm && !options.update && this[_global]) { - const er = new Error('global requires add, rm, or update option') - return Promise.reject(er) + throw new Error('global requires add, rm, or update option') } // first get the virtual tree, if possible. If there's a lockfile, then @@ -334,6 +333,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { root.meta.lockfileVersion = defaultLockfileVersion } } + root.meta.inferFormattingOptions(root.package) return root }) diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js index a7a68c98c6d6d..b45fea0ac6111 100644 --- a/workspaces/arborist/lib/shrinkwrap.js +++ b/workspaces/arborist/lib/shrinkwrap.js @@ -424,6 +424,18 @@ class Shrinkwrap { .map(fn => fn && maybeStatFile(fn))) } + inferFormattingOptions (packageJSONData) { + // don't use detect-indent, just pick the first line. + // if the file starts with {" then we have an indent of '', ie, none + // which will default to 2 at save time. + const { + [Symbol.for('indent')]: indent, + [Symbol.for('newline')]: newline, + } = packageJSONData + this.indent = indent !== undefined ? indent : this.indent + this.newline = newline !== undefined ? newline : this.newline + } + load () { // we don't need to load package-lock.json except for top of tree nodes, // only npm-shrinkwrap.json. @@ -451,15 +463,7 @@ class Shrinkwrap { return data ? parseJSON(data) : {} }).then(async data => { - // don't use detect-indent, just pick the first line. - // if the file starts with {" then we have an indent of '', ie, none - // which will default to 2 at save time. - const { - [Symbol.for('indent')]: indent, - [Symbol.for('newline')]: newline, - } = data - this.indent = indent !== undefined ? indent : this.indent - this.newline = newline !== undefined ? newline : this.newline + this.inferFormattingOptions(data) if (!this.hiddenLockfile || !data.packages) { return data diff --git a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs index ca47bbbe1b135..d8f4fc4d52b1e 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs @@ -97673,6 +97673,37 @@ ArboristNode { } ` +exports[`test/arborist/build-ideal-tree.js TAP store files with a custom indenting > must match snapshot 1`] = ` +{ + "name": "tab-indented-package-json", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "tab-indented-package-json", + "version": "1.0.0", + "dependencies": { + "abbrev": "^1.0.0" + } + }, + "node_modules/abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + } + }, + "dependencies": { + "abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + } + } +} + +` + exports[`test/arborist/build-ideal-tree.js TAP tap vs react15 > build ideal tree with tap collision 1`] = ` ArboristNode { "children": Map { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 80db25880e46c..8ec8dab914e35 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -3682,3 +3682,16 @@ t.test('overrides', t => { t.end() }) + +t.test('store files with a custom indenting', async t => { + const tabIndentedPackageJson = + fs.readFileSync( + resolve(__dirname, '../fixtures/tab-indented-package-json/package.json'), + 'utf8' + ).replace(/\r\n/g, '\n') + const path = t.testdir({ + 'package.json': tabIndentedPackageJson, + }) + const tree = await buildIdeal(path) + t.matchSnapshot(String(tree.meta)) +})