Npm audit fix

[issackjohn]

This PR updates the package-lock.json files for the affected packages by running npm audit fix to address CVE-2023-26159.

• Rolls follow-redirects to 1.15.5 instead of 1.15.4 as mentioned in a comment the issue.
• Rebuilds all dists where package-lock.json was touched.

Hosted at: https://issackjohn.github.io/Speedometer3

closes #355

[flashdesignory]

npm audit fix caught a few more packages that needed updating 👍

[julienw]

GIven all the changes, I'm wondering that wemay need to regenerate all dists as well. What do you think?

[flashdesignory]

GIven all the changes, I'm wondering that wemay need to regenerate all dists as well. What do you think?

I had that as a todo item ... not sure if @issackjohn wants to run the builds for these anyways and I can do the same with the few that weren't touched afterwards?

[julienw]

I think it would be good to do that here to make sure that they still build after the changes.

[issackjohn]

I think it would be good to do that here to make sure that they still build after the changes.

Rebuild all the dists which had package-lock.json updated in this PR right?

[issackjohn]

GIven all the changes, I'm wondering that wemay need to regenerate all dists as well. What do you think?

I had that as a todo item ... not sure if @issackjohn wants to run the builds for these anyways and I can do the same with the few that weren't touched afterwards?

Looks like news-next doesn't work on github pages. @flashdesignory Would you be able to help me collect the before and after numbers for that? I can do the others.

[issackjohn]

5 runs

Other view:
Before:

After:

[julienw]

I notice that jQuery (both simple and complex versions) shows quite a big increase on all browsers, do we know why?

[julienw]

It could come from the handlebars update. We were previously using a very old version from 2017.

[julienw]

Thanks a lot for this update!

I checked a few workloads that all seem to work. I noticed #363 in jQuery, but it looks like this is preexisting, so let's not think about it here.

I believe we can live with the regression on jquery, what do you think @bgrins?

I see that the workloads in editors/ and charts/ would need their npm audit fix run too. Do you think you could handle that as well, in a separate PR?

react-stockcharts is deeply unmaintained but I don't think we can update that one without breaking it in its current state, so I'd skip it.

[smaug----]

This affects performance numbers a lot, so we should know more why that is happening. I don't think we should land this before there has been some more investigation on that.

[issackjohn]

I would like to scope this. The intentions of my change were to update the follow-redirects package version to conform with internal compliance. Perhaps we could upgrade the other packages of jQuery in a separate PR? That way we don't block this one.

[bgrins]

I would like to scope this. The intentions of my change were to update the follow-redirects package version to conform with internal compliance. Perhaps we could upgrade the other packages of jQuery in a separate PR? That way we don't block this one.

That seems fine to me - let's get this one closed with the development environment specific updates

[bgrins]

It could come from the handlebars update. We were previously using a very old version from 2017.

This affects performance numbers a lot, so we should know more why that is happening. I don't think we should land this before there has been some more investigation on that.

I agree. We intentionally decided on framework versions for 3.0, and changing them should be deliberately considered - not based on the output of the audit command. We can consider whether updating this (along with other frameworks) is a good idea in a future version, there's plenty else to do now.

[bgrins]

Thanks for the update Issack. Looking at the new diff, the following dist/ directories appear to have nontrivial changes:

• newssite-next
• react-complex
• react-redux-complex
• react-redux
• react
• vue-complex
• vue

Do we know if these are latent differences on main between what's in dist/ and the output of npm run build, or are these changes as a result of something in the audit fix? Because presumably updating follow-redirects should result in no changes to any dist.

[issackjohn]

Before

After:

[issackjohn]

Thanks for the update Issack. Looking at the new diff, the following dist/ directories appear to have nontrivial changes:

• newssite-next
• react-complex
• react-redux-complex
• react-redux
• react
• vue-complex
• vue

Do we know if these are latent differences on main between what's in dist/ and the output of npm run build, or are these changes as a result of something in the audit fix? Because presumably updating follow-redirects should result in no changes to any dist.

The result of npm audit and then rebuilding the dists. You're saying that we should only run npm update follow-redirects in each of the affected packages of #351 right?

[julienw]

Besides jQuery with handlebars, I think that the other changes are fairly trivial. The large amount of upgraded packages comes mostly from babel.
Have you seen different results in the packages you mentioned @bgrins ?

[bgrins]

The result of npm audit and then rebuilding the dists. You're saying that we should only run npm update follow-redirects in each of the affected packages of #351 right?

My suggestion is that we do two separate steps: first we just go through the existing directories and do npm run build to start with a baseline of the current tree with no package.json changes - this is what #346 was opened for.

Then separately, we would do some kind of package.json updates (either the approach here or the approach in #351 - I don't have a strong opinion about this and haven't closely been following the discussion, but presumably the current approach in this PR is good), and do npm run build again. That way we'll know exactly what is causing changes to the dist directory.

[bgrins]

Besides jQuery with handlebars, I think that the other changes are fairly trivial. The large amount of upgraded packages comes mostly from babel.
Have you seen different results in the packages you mentioned @bgrins ?

It's hard for me to tell what the substance of the changes are (it's hard to tell from the PR diff, and why I think it would be helpful to "clear out" any latent differences with main dist/ directories and the result of the build). But I'll defer to you on the review here: if you and others are satisfied with the current diff it's fine with me

[julienw]

GitHub has a nice viewer for the package-lock.json, you can click the button with the "document" icon.
I believe we can revert the update of handlebars in jQuery.
I'll look closely at the others on Monday, see if there's anything other than devDependencies.

[julienw]

Thanks for changing the jquery workload so that just "follow-redirects" is updated there.

I looked at all other updates, they're all dev dependencies: mostly babel, webpack, and related packages such as postcss. The angular package got some updates for angular-related packages too, but only build-related, not runtime.

Can you please provide new numbers in the various browsers so that we can see if the regression is still present?
Thanks!

[issackjohn]

Before

After:

@julienw these are the new numbers after the commit that only changed jQuery

[issackjohn]

Before:

After:

[julienw]

OK, this looks good to me! Thanks for all the updates

[issackjohn]

OK, this looks good to me! Thanks for all the updates

Thanks!

[issackjohn]

@rniwa PTAL

[issackjohn]

Thank you all for your reviews and help!