fix npm warnings #16579

AritraDey-Dev · 2025-06-13T12:18:51Z

Description

This PR resolves npm warnings caused by deprecated glob@7.2.3 and inflight@1.0.6 by overriding them.
few observations:

Before

$ npm explain inflight glob
inflight@1.0.6
node_modules/inflight
  inflight@"^1.0.4" from glob@7.2.3
  node_modules/glob
    glob@"^7.2.0" from @babel/cli@7.23.0
    node_modules/@babel/cli
      @babel/cli@"^7.23.0" from the root project
    glob@"^7.2.0" from svgstore-cli@2.0.1
    node_modules/svgstore-cli
      svgstore-cli@"^2.0.1" from the root project

glob@7.2.3
node_modules/glob
  glob@"^7.2.0" from @babel/cli@7.23.0
  node_modules/@babel/cli
    @babel/cli@"^7.23.0" from the root project
  glob@"^7.2.0" from svgstore-cli@2.0.1
  node_modules/svgstore-cli
    svgstore-cli@"^2.0.1" from the root project

Now (After the fix)

$  npm explain inflight glob
glob@10.4.5
node_modules/glob
 overridden glob@"^10.0.0" (was "^7.2.0") from @babel/cli@7.27.2
 node_modules/@babel/cli
   @babel/cli@"^7.27.2" from the root project
 overridden glob@"^10.0.0" (was "^7.2.0") from svgstore-cli@2.0.1
 node_modules/svgstore-cli
   svgstore-cli@"^2.0.1" from the root project

Fixes #16573

Reviewers

Fixes istio#16573 Signed-off-by: Aritra Dey <adey01027@gmail.com>

dhawton · 2025-06-13T13:40:46Z

Why not place the override directly in packages.json? I don't like having things be different in tests vs deployment.. they should be the same.

AritraDey-Dev · 2025-06-13T14:00:43Z

Why not place the override directly in packages.json? I don't like having things be different in tests vs deployment.. they should be the same.

Yes, my first thought was to do that in package.json itself. But there is no package.json in the project — it is created by the Makefile.core.mk here

istio.io/Makefile.core.mk

Line 140 in 95c8d11

@npm init -y

So if we need to make changes to package.json, we first need to create it and then apply the override.

dhawton · 2025-06-13T14:25:17Z

Might be worth changing... I don't like the thought of having production be different from dev. I'll defer to @craigbox here though as there are a number of plans he has for docs that I am not in the loop on.

AritraDey-Dev · 2025-06-13T14:44:16Z

yes i agree it’s better to have a consistent package.json checked into the project so it don’t have differences between dev and prod.

craigbox · 2025-06-17T09:24:11Z

If we need the files to exist, then we should do it the idiomatic way.
Could we then have the Dockerfile include the dependencies from the packages.json file instead?

(It might be worth a quick look through the code history to see if we can figure out why it wasn't done like that from the start)

AritraDey-Dev · 2025-06-17T18:26:15Z

After looking at the previous commit logs, it seems that PR #3242 removed package.json, and then they used npm install separately for each package.
Later, the Docker build process was migrated to the istio/tools repository via PR #4722.

I think the best approach would be to add the overrides in the Dockerfile of the tools repository as well — specifically here:
https://github.com/istio/tools/blob/97af7d3c6f6057a41dab45e8780a6d3d56820f15/docker/build-tools/Dockerfile#L463-L479
By adding the following line:

@jq '. + {overrides: {"glob": "^10.0.0", "inflight": false}}' package.json > tmp.$$ && mv tmp.$$ package.json

This would make the behavior consistent across both dev and prod environments, addressing @dhawton's concern.
@craigbox Would you agree that this is the right step to take next?

craigbox · 2025-06-17T23:02:54Z

It looks like Netlify used to use the website builder container?
Don't lean into the history too much here. We do things how we do them today. It may be that the change Martin made back then was probably to get the most recent versions of each package installed each time. We don't want that, and I don't see a compelling reason not to use packages.json at this point.

The principle of simplicity for future maintainers should apply - I don't think that a JQ line and temporary generated files achieves that.

(An even more idiomatic answer might be hugo mod npm pack and packages.hugo.json but I think that's for a later date)

craigbox · 2025-06-18T22:43:04Z

(Daniel had a similar thought here.)

AritraDey-Dev · 2025-06-19T18:08:24Z

Hmm, your point makes sense—having a package.json is a good idea. In the future, other packages might also require overrides, so I'll add those files.

Signed-off-by: Aritra Dey <adey01027@gmail.com>

AritraDey-Dev · 2025-06-19T19:00:31Z

package.json

@@ -0,0 +1,38 @@
+{
+  "name": "work",


Since the package.json is generated by npm init -y, the name is still set to "work". Should we change it to "istio.io", or is it okay to keep it as is?

I would change it

Ajay-singh1 · 2025-06-20T00:02:56Z

Thanks for your effort, but I wanted to clarify that the current pipeline is already clean after PR #16574 where we added esbuild as a bundler & minifier.Their are no existing errors and warning everything is neat and clean.

Suppressing or overriding warnings globally could mask future issues and is not an ideal long term solution.Let's focus on meaningful improvement that genuinely help the project.

Thanks.

AritraDey-Dev · 2025-06-20T01:10:39Z

I believe both issues are different. This PR attempts to address the warnings caused by deprecated glob and inflight.
I suggest running make netlify on your PR branch
— you will see the warnings related to glob and inflight,

npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported

which are also mentioned in issue #16573.
So, even if we switch to esbuild, these warnings will still persist unless we use some versions of glob or inflight that do not trigger the deprecation warnings.

Ajay-singh1 · 2025-06-20T02:28:47Z

No need to override , these are transitive dependency and the warning is caused by the deprecated svgstore-cli npm package you can replace it with modern npm packages.

See:-

AritraDey-Dev · 2025-06-20T03:08:12Z

In your previous comment, you claimed that your PR fixes this issue too — I think it's clear now that it doesn't.
Regarding the overrides: sure, switching to other packages might be better, but that’s something we should confirm with the maintainers first. And in this PR if you check the earlier comments, the idea was to keep overrides in package.json and so followed that approach

Ajay-singh1 · 2025-06-20T03:28:34Z

Sure! I'm not in favour of suppressing this warnings also overriding npm warnings especially for deprecated or unmaintained dependency - can hide known vulnerabilities (CVEs).

I would also like to escalate to maintainers for the best path forward.

craigbox · 2025-06-23T00:13:39Z

Makefile.core.mk

 	    sass@v1.89.1 \
-	    typescript@v5.8.3 \
-	    svgstore-cli@v2.0.1 \
-		@babel/core@v7.27.4 \
-		@babel/cli@v7.27.2 \
-		@babel/traverse@7.25.9 \
-		@babel/preset-env@v7.27.2
-	@npm install --omit=dev --save-dev \
-		babel-preset-minify@v0.5.2
-	@npm install --save \
-		core-js@3.42.0
+	    typescript@v5.8.3 


Why would these not be in packages.json?

Because these packages need to be installed globally, we can either include them in the Makefile as we do now, or add them to the scripts section of package.json.

craigbox · 2025-06-23T00:13:48Z

Makefile.core.mk

@@ -140,16 +140,7 @@ netlify_install:
 	@npm init -y


would this still be needed?

Just kept it for now as netlify still depends on it...can be replaced by npm install once package.json Is created

craigbox · 2025-06-23T00:13:55Z

package.json

@@ -0,0 +1,38 @@
+{
+  "name": "work",


I would change it

craigbox · 2025-06-23T00:14:00Z

package.json

+{
+  "name": "work",
+  "version": "1.0.0",
+  "description": "| Site | Status |------|------- | istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/c98435af-5464-4ac3-93c2-9c98faeec9b6/deploy-status)](https://app.netlify.com/sites/istio/deploys) | preliminary.istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/a1cfd435-23d5-4a43-ac6d-8ec9230d9eb3/deploy-status)](https://app.netlify.com/sites/preliminary-istio/deploys) | archive.istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/f8c3eecb-3c5c-48d9-b952-54c7ed0ece8f/deploy-status)](https://app.netlify.com/sites/archive-istio/deploys)",


...and this

Entire package.json is generated by npm init -y .so I didn't touch it yet.

craigbox · 2025-06-23T00:14:05Z

package.json

+  "description": "| Site | Status |------|------- | istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/c98435af-5464-4ac3-93c2-9c98faeec9b6/deploy-status)](https://app.netlify.com/sites/istio/deploys) | preliminary.istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/a1cfd435-23d5-4a43-ac6d-8ec9230d9eb3/deploy-status)](https://app.netlify.com/sites/preliminary-istio/deploys) | archive.istio.io | [![Netlify Status](https://api.netlify.com/api/v1/badges/f8c3eecb-3c5c-48d9-b952-54c7ed0ece8f/deploy-status)](https://app.netlify.com/sites/archive-istio/deploys)",
+  "main": "index.js",
+  "directories": {
+    "test": "tests"


we don't have any of these?

#16579 (comment)
Same for this also

craigbox · 2025-06-23T00:14:15Z

package.json

+  },
+  "keywords": [],
+  "author": "",
+  "license": "ISC",


craigbox · 2025-06-23T00:14:34Z

package.json

+    "core-js": "^3.42.0",
+    "svgstore-cli": "^2.0.1"
+  },
+  "devDependencies": {


why is this a devDependency and not a dependency, do you know?

istio.io/Makefile.core.mk

Line 149 in b5c0c98

@npm install --omit=dev --save-dev \

the --save dev flag install it as a devDependencies.
And this dependencies are required for build only not for runtime,so often used as devDependencies.
can be put inside dependencies section,but for sure keeping it in devDependencies is preferred always

craigbox · 2025-06-23T00:15:49Z

Sure! I'm not in favour of suppressing this warnings also overriding npm warnings especially for deprecated or unmaintained dependency - can hide known vulnerabilities (CVEs).

I would also like to escalate to maintainers for the best path forward.

We are pinning the version of the svgstore package so it's reasonable to also pre-acknowledge the warnings at that version. I'm happy to update to something better supported if we can find something (discussed in the other PR).

AritraDey-Dev · 2025-07-02T19:55:37Z

Once the esbuild PR is merged, and since we have already migrated from svgstore to svg-sprite-generator, do we still need the package.json file for the project?
If not, I believe this can be closed?

istio-testing · 2025-07-03T11:55:17Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

chore: fix npm warnings

7e156f1

Fixes istio#16573 Signed-off-by: Aritra Dey <adey01027@gmail.com>

AritraDey-Dev requested a review from a team as a code owner June 13, 2025 12:18

istio-policy-bot added the area/environments label Jun 13, 2025

istio-testing added the size/XS label Jun 13, 2025

AritraDey-Dev changed the title ~~chore: fix npm warnings~~ fix npm warnings Jun 13, 2025

AritraDey-Dev added 2 commits June 20, 2025 00:01

removed overrides from netlify_install

f6a2d90

Signed-off-by: Aritra Dey <adey01027@gmail.com>

added package.json files

62762bc

Signed-off-by: Aritra Dey <adey01027@gmail.com>

istio-testing added size/XXL and removed size/XS labels Jun 19, 2025

AritraDey-Dev commented Jun 19, 2025

View reviewed changes

AritraDey-Dev mentioned this pull request Jun 22, 2025

migrate svgstore to svg-sprite-generator #16598

Merged

11 tasks

craigbox reviewed Jun 23, 2025

View reviewed changes

istio-testing added the needs-rebase label Jul 3, 2025

Ajay-singh1 closed this Jul 5, 2025

fix npm warnings #16579

fix npm warnings #16579

Uh oh!

Conversation

AritraDey-Dev commented Jun 13, 2025

Description

Before

Now (After the fix)

Reviewers

Uh oh!

dhawton commented Jun 13, 2025

Uh oh!

AritraDey-Dev commented Jun 13, 2025

Uh oh!

dhawton commented Jun 13, 2025

Uh oh!

AritraDey-Dev commented Jun 13, 2025

Uh oh!

craigbox commented Jun 17, 2025

Uh oh!

AritraDey-Dev commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

craigbox commented Jun 17, 2025

Uh oh!

craigbox commented Jun 18, 2025

Uh oh!

AritraDey-Dev commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ajay-singh1 commented Jun 20, 2025

Uh oh!

AritraDey-Dev commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ajay-singh1 commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AritraDey-Dev commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ajay-singh1 commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

craigbox commented Jun 23, 2025

Uh oh!

AritraDey-Dev commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-testing commented Jul 3, 2025

Uh oh!

Uh oh!

AritraDey-Dev commented Jun 17, 2025 •

edited

Loading

AritraDey-Dev commented Jun 20, 2025 •

edited

Loading

Ajay-singh1 commented Jun 20, 2025 •

edited

Loading

AritraDey-Dev commented Jun 20, 2025 •

edited

Loading

AritraDey-Dev commented Jul 2, 2025 •

edited

Loading