Framework: Avoid churn in npm-shrinkwrap.json

[nylen]

Our npm-shrinkwrap.json which lists the exact versions of all of our dependencies has gotten a bit crazy. Here is a history of commits that changed this file with the number of lines added/removed in each commit:

$ git log --oneline npm-shrinkwrap.json | cut -d' ' -f1 | while read i; do git show --text "$i" npm-shrinkwrap.json | grep -Po '^[+-]' | sort | uniq -c | tr -d '\n'; echo -n '    '; git log --oneline "$i" -n 1 | cat; done | pbcopy
    808 -     17 +    de96853 Bump to Gridicons 3.0.1 (#23306)
      3 -    794 +    4901875 package.json: Bump `notifications-panel` to v2.1.8 (#23278)
      9 -      9 +    52b3577 Revert "Update to gridicons 3 (#23171)" (#23304)
    801 -     22 +    113841d Update to gridicons 3 (#23171)
    167 -    377 +    0a930af Framework: Use qs module instead of querystring (#23265)
     25 -     24 +    0fceb6d package.json: Bump `notifications-panel` to v2.1.6. (#23182)
     76 -     90 +    a5ea162 PR Tweak: Less hacky approach with an extended DelayRender component
    249 -    255 +    28febda Upgrade moment library to 2.21.0 (#23023)
      8 -    176 +    a560773 Webpack: strictly verify imports and disable nodejs polyfilling (#22630)
    123 -   1274 +    6eb73cd Dependencies: Bump wpcom.js and slightly improve webpack config (#22854)
     95 -     84 +    366ad7b Framework: Update Gridicons to v.2.1.3 (#22732)
    202 -     22 +    de953d7 Framework: eliminate pug (#22348)
    100 -    894 +    eb1ed86 Remove dependency on the 'numeral' NPM package
   1094 -    309 +    74ed256 Fix the "analyze-bundles" and "build-client" commands in Windows
... and many many more

You can see that there is a lot of churn in this file recently. Some of it is related to intentional dependency changes, but there are also these changes of about 800 lines that flip back and forth fairly consistently.

On my computer, depending on how I update the lock file in master, I can get one of several different results:

    738 -    118 +    093f7c7 Update shrinkwrap again (with --no-optional flag)
    114 -    734 +    24b4a25 Update shrinkwrap using `npm run update-deps`
     10 -     10 +    e23c7cf Update shrinkwrap using `npm run update-deps` (3rd time's the charm!)

This file changes dramatically depending on who updates it, apparently depending on the OS and configuration they are using to develop Calypso. For example: the optional dependency fsevents is only installed on OS X, so my computer doesn't install it. Some history on this issue: #12370 (comment), #18767 (comment)

But ideally, we just want a package lock file that ensures reliable builds in Docker and on the Calypso servers, so it shouldn't change across different kinds of machines or configurations. We also shouldn't be committing a lot of these spurious diffs to git.

I'm not sure the best way to fix or improve this, but seeing big npm-shrinkwrap.json diffs in PRs makes me a bit nervous.

[blowery]

Possible solutions:

• Find a way to run update-deps inside docker. Likely to work, unsure how difficult.
• Move to yarn and yarn's lockfile. Might have the same issues?
• Move to package.lockfile from recent NPMs. Probably doesn't help. This was discussed previously.

[blowery]

https://codingwithspike.wordpress.com/2017/08/11/why-im-sticking-with-yarn/
Interesting article on how yarn deals with optional deps. Sounds more stable.

[samouri]

Looks like this was all fixed in npm v5.6: http://blog.npmjs.org/post/167963735925/v560-2017-11-27

We can also take advantage of a new option for npm run update-deps that only generates the lockfile without messing with node_modules!

[blowery]

@samouri my between-the-lines read of that is that we also have to move to a package-lock.json, off of shrinkwrap?

[samouri]

@samouri my between-the-lines read of that is that we also have to move to a package-lock.json, off of shrinkwrap?

We don't need to but it would be good to do. Since shrinkwrap and package-lock are the exact same file format, we could just rename the lock file once its created.

[blowery]

Since shrinkwrap and package-lock are the exact same file format, we could just rename the file once its created.

Right now we do some post-processing on the shrinkwrap to let it work with the npm mirror we use when building prod images... We'll probably need to continue doing that?

[scinos]

This is not relevant anymore since we move to yarn.