[arborist] [Fix] build-ideal-tree: ensure indentation is preserved

[ljharb]

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. I also included a bonus commit that removes redundant Promise stuff inside an async function.

I'm happy to add a test if you suggest where to do so; it seems like it'd require a directory with a package.json that's indented by tabs, and to confirm that the resulting shrinkwrap contents is also indented by tabs.

(related to #4181)

[ruyadorno]

thanks @ljharb! these looks like good catches to me 😊

With regards to the lockfile format, I would prefer to have a more resilient implementation in place while also avoiding some of the unintended repetition. With that in mind I'd favor more refactoring this bit of code from lib/shrinkwrap.js into its own public method e.g: format(source) so that we can make sure we're guarding against undefined values while also making sure we copy over the line break characters:

cli/workspaces/arborist/lib/shrinkwrap.js

Lines 454 to 462 in aa538df

	// 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 way in L337 over here you can just reuse that same root.meta.format(root.package)

This way it's also going to be simpler adding a test to test/shrinkwrap.js that just validates the format() method works as intended. There's already a tab-indented-package-json fixture that can be used in it. I can help with more guidance on how to implement the test if needed so.

Let me know what you think 😄 and thanks again!

[ljharb]

hmm, i'm not sure i understand. The actual formatting isn't being done here - that's just setting the this.indent and this.newline for later usage.

[ljharb]

With the recent updates, I can confirm that buildIdealTree is indeed inferring the indentation properly in my npm-lockfile project.

[ruyadorno]

awesome 🥳 thank you @ljharb!