Skip to content

Commit

Permalink
fix(arborist): ensure indentation is preserved (#4218)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
ljharb authored Jan 18, 2022
1 parent a92665c commit 510f0ec
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 12 deletions.
6 changes: 3 additions & 3 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -334,6 +333,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
root.meta.lockfileVersion = defaultLockfileVersion
}
}
root.meta.inferFormattingOptions(root.package)
return root
})

Expand Down
22 changes: 13 additions & 9 deletions workspaces/arborist/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})

0 comments on commit 510f0ec

Please sign in to comment.