Ensure hpal new maintains alphabetical order of deps lists

[zemccartney]

This work addresses #38

Alphabetically sort keys that we can reasonably guess are dependencies or devDependencies (I say guess b/c it's possible to trick those guesses if keys in other nodes of package.json match those in dependencies or devDependencies e.g. if there were scripts named confidence and eslint, for example). This is imprecise, but I went this way to try to avoid alphabetizing the non-dependency list "2nd-level" keys e.g. scripts (I tried a regex replace and reparse approach on just our dependencies, which felt a little more precise, but this fell down b/c i found it impossible to get the indentation of the replaced text right. This killed me inside a bit 🤡 )

Clearly, I bailed on testing thoroughly :) I noted in comments why I turned coverage off in the changes to the stable-stringify comparison function. I think the main issue with testing those cases is I couldn't figure out how to replace the boilerplate's package.json with an arbitrary file, one I could build to trigger the cases uncovered by the original package.json. I bailed on this for now since it seemed tricky enough that it could be a time sink and I'd rather first confirm with you this solution seems sane enough before proceeding with any testing deepdive.

Background, if helpful

I don't entirely know why this issue was happening, but, as @devinivy noted here, this appears to be due to reliance on something in v8's internals. Specifically, it looks like node 12 and 10 differ in how they preprocess an input array for sort e.g. for the boilerplate's list of devDependencies, running devDepsNames.sort(() => 0) (i.e. do not change list order) on both versions yields alphabetical results in node 12 and the non-alphabetized list seen on the linked issue. As for WHY node 10 does this, I haven't the slightest :)

Further reading if it helps. I don't understand these resources much at all, just enough to takeaway that sorting in node 12 is expectedly different from 10 🎺 :

• Array.sort arguments order changed in Node.js 11 nodejs/node#24294
• https://v8.dev/blog/array-sort

[coveralls]

Coverage remained the same at 100.0% when pulling c7308bb on order-deps into 7480a4b on master.

[devinivy]

This is v good stuff, thank you! The main snag that I see is if there are multiple dependencies which have the same name as the top level of the package.json, which is rather unlikely. Of course the coverage wart sort of goes along with that. And it's really all due to the approach that I originally took, prior to your work here.

I am down to accept this as-is since it's a good practical fix to a rather annoying problem for newcomers. It also got me thinking: npm must deal with this too and we're trying to interop with npm, so how do they deal with it?. What I learned (starting roughly here) is that they achieve stable json-stringificaiton simply by creating objects and inserting keys in the order they want using e.g. sorted-object. That means sorting the package.json root object separately from the dependency objects, whereas here we have to do both at the same time, which causes the snag mentioned above.

That is all to say, I would like to take another pass at this at some point to bring it more in line with the npm CLI, but this is great for now and thanks again!!

[zemccartney]

Hey, thanks for reviewing all this. And thanks for doing that research and laying out your findings. That's super cool; of course npm would have thought of these problems 😛 Really cool solution there.

I'm totally down to take another pass at this (whether holding this up or as a separate PR), aiming toward something more akin to npm's approach, if trying to do this more righter now feels like the best move. I can't promise too much about timeline, tho :)

Would you mind if I took another shot? Or rather just close out for now, decide next steps later?

[devinivy]

Sure thing, by all means take another pass at it. If it's going to take a while that's all cool, and I will just release this fix as-is for the time being. Let's re-evaluate that in a few days.

[zemccartney]

wicked, sounds great. really appreciate it, my dude! 🍻

[zemccartney]

@devinivy h'ok! just pushed up an attempt at a strategy similar to npm's for building package.json

Side-note: To test, I checked if this new version avoided the cherry-picking conflict we see when applying the deployment flavor (on node 10); just FYI, looks like the deployment flavor is out of date with the others, didn't receive the babel eslint update. That said, with these changes and manually clearing babel-eslint from the output package.json before cherry-picking, the flavor is applied cleanly. My bad if you were already aware and I'm just squawkin' over here! 🙏

[devinivy]

What is actionable for this TODO? Let's resolve this!

[zemccartney]

🤦 lol, shit. I'd meant to mention this, even peeked that project's license, then promptly spaced. cool! Solution below looks chill!

[devinivy]

Aside from the pending TODO, I am into this! Out of curiosity, were you able to verify the test failed on node v10 and was resolved with these changes?

[devinivy]

@zemccartney thanks for flagging that re: the deployment flavor! Not sure how that got passed-over. Should be fixed.

[zemccartney]

Wicked, thanks for the review and looking into the deployment flavor! (And for remembering to merge master (lol, sorry 😏 ))

Yes, I verified this test fails on node 10 w/o these changes and is fixed with them. At least, I did the following (just re-ran these steps this morning, pulling down the latest)

1. npx hpal new projy, init commit
2. git cherry-pick deployment
3. Then, in my local copy of hpal, node bin/hpal new projy, init commit
4. git cherry-pick deployment

I saw the following conflict after step 2

"devDependencies": {
    "babel-eslint": "10.x.x",
    "@hapi/code": "8.x.x",
    "@hapi/eslint-plugin-hapi": "4.x.x",
    "@hapi/lab": "22.x.x",
<<<<<<< HEAD
    "@hapi/eslint-config-hapi": "13.x.x",
    "confidence": "4.x.x",
    "dotenv": "8.x.x",
=======
    "babel-eslint": "10.x.x",
>>>>>>> cb0b495... (flavor) deployment v2.5.0
    "eslint": "6.x.x",
    "hpal": "2.x.x",
    "hpal-debug": "1.x.x"
  }

But step 4 was clean, showed the deployment commit appended to the project's commit log