Skip to content

Rebuild package lock - May 2020 #4802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 2, 2020
Merged

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented May 1, 2020

In respect to ##4796
This PR helps reduce the number of warnings namely for kind-of package.

Before:
before

After:
after

See commit a883997 for details.

Also to mention, the audit files were created using the commands below:

npm audit --json > audit.json
npm audit | sed -r "s/\x1B\[([0-9]{1,3}(;[0-9]{1,2})?)?[mGK]//g" > audit.txt

@plotly/plotly_js

@archmoj archmoj added this to the v1.54.1 milestone May 1, 2020
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Looks good. Just curious, do you have any idea what the bundled: true flags are, and why they got stripped out and replaced by resolved and integrity keys in a bunch of places?

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

Here is relavant info:
https://docs.npmjs.com/configuring-npm/package-lock-json.html#bundled
https://docs.npmjs.com/configuring-npm/package-lock-json.html#integrity

@antoinerg @alexcjohnson
Do you get an identical package-lock.json file on mac/windows using node 12?

@alexcjohnson
Copy link
Collaborator

Mac OS 10.15.4, on this branch, substantial changes:

> node --version
v12.13.1
> npm --version
6.13.7
> rm package-lock.json
> rm -rf node_modules
> npm i
> git diff --stat
 package-lock.json | 4405 ++++++++++++++++++++---------------------------------
 1 file changed, 1620 insertions(+), 2785 deletions(-)
> npm audit
found 15 vulnerabilities (7 low, 8 moderate) in 13866 scanned packages

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

@alexcjohnson thanks for testing this out. Do you get any diff when calling git status after npm i?
I'd like to know we get an identical package-lock.json file.

@alexcjohnson
Copy link
Collaborator

yes, that 1620 insertions(+), 2785 deletions(-) is vs the tip of this branch.

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

You get different lock file?

@alexcjohnson
Copy link
Collaborator

Much closer - now if I do the same reinstall vs your latest commit I get:

 package-lock.json | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Specifically:

diff --git a/package-lock.json b/package-lock.json
index 6e0131d..b447142 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -875,6 +875,16 @@
       "resolved": "https://registry.npmjs.org/binary-search-bounds/-/binary-search-bounds-2.0.4.tgz",
       "integrity": "sha512-2hg5kgdKql5ClF2ErBcSx0U5bnl5hgS4v7wMnLFodyR47yMtj2w+UAZB+0CiqyHct2q543i7Bi4/aMIegorCCg=="
     },
+    "bindings": {
+      "version": "1.5.0",
+      "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz",
+      "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==",
+      "dev": true,
+      "optional": true,
+      "requires": {
+        "file-uri-to-path": "1.0.0"
+      }
+    },
     "bit-twiddle": {
       "version": "1.0.2",
       "resolved": "https://registry.npmjs.org/bit-twiddle/-/bit-twiddle-1.0.2.tgz",
@@ -4040,6 +4050,13 @@
       "integrity": "sha512-r70c72ln2YHzQINNfxDp02hAhbGkt1HffZ+Du8oetWDLjDtFja/Lm10lUaSh9e+wD+7VDvPee0b0C9SAy8pWZg==",
       "dev": true
     },
+    "file-uri-to-path": {
+      "version": "1.0.0",
+      "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz",
+      "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==",
+      "dev": true,
+      "optional": true
+    },
     "filing-cabinet": {
       "version": "2.5.1",
       "resolved": "https://registry.npmjs.org/filing-cabinet/-/filing-cabinet-2.5.1.tgz",
@@ -11795,6 +11812,7 @@
           "dev": true,
           "optional": true,
           "requires": {
+            "bindings": "^1.5.0",
             "nan": "^2.12.1",
             "node-pre-gyp": "*"
           },

Maybe these are Mac-only requirements... doesn't bother me. The test failures do though 🙄

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

The tests are green and the warnings even reduced to 15.
newAfter

Could you please try npm i this time without removing package-lock.json then git status to see if there are any changes to package-lock.json?

@alexcjohnson
Copy link
Collaborator

If I run npm i without removing node_modules, I again get bindings and file-uri-to-path added to package-lock. But if I remove node_modules first, then run npm i, I get no changes to package-lock at all 🎉

Your branch is up-to-date with 'origin/rebuild-package-lock-May01-2020'.
nothing to commit, working tree clean

This looks great, but what does not allowing any bump by removing ^ before npm i mean? What command(s) did you run?

@alexcjohnson
Copy link
Collaborator

Anyway we don't need to answer every last question about package-lock tonight 😅 This is fantastic, the 💃 above still applies 🥇

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

Thanks very much @alexcjohnson.
Have a good and JS-less week-end.

@archmoj
Copy link
Contributor Author

archmoj commented May 2, 2020

This looks great, but what does not allowing any bump by removing ^ before npm i mean? What command(s) did you run?

To regenerate package-lock, I first removed package-lock;
then removed ^ characters in front of versions noted in package.json file but I didn't commit that file.
Then npm i created a new package-lock` which I did commit.
Finally I spent time looking at the diff between previous build and the new build (not committed) to find the packages that contributed in the changes to the source.
Those appear to be

binary-search-bounds
d
d3-collection
d3-color
d3-path
d3-shape
es6-iterator
es6-symbol
es6-weak-map
glsl-tokenizer
array-normalize
ndarray
regl-splom
svg-arc-to-cubic-bezier
to-px

Then run npm ls each package once on master, and once on this branch.
There were few which we may want not to update at the moment e.g. to-px and ndarray. So the versions are locked in 6032aba.

I will move forward and merge this thing.

@archmoj archmoj merged commit cbd7e6e into master May 2, 2020
@archmoj archmoj deleted the rebuild-package-lock-May01-2020 branch May 2, 2020 05:50
This was referenced May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants