Update NPM packages

[angelocordon]

What type of PR is this? (check all applicable)

• 🎨 Enhancement
• 🚩 Other (chores)

Context

Update NPM packages to handle vulnerability issues. Also updated packages that are way behind on their versions.

Implementation Details - what was your thought process as you changed the code?

1/ To find packages that are currently vulnerable, run npm audit. Running npm audit --fix can fix most of the issues. NPM itself (listed in our package.json) was not automatically fixed and had to be updated by running npm update npm@latest.

2/ Running npm outdated also gives us a list of packages that are way behind from their current release versions. Updated packages based on themes instead of updating all at once to see if it breaks anything. In between the package updates, I ran the tests to see if what we have still passed (although our test coverage is kind of lacking). While the packages were not updated to their latest available versions, their current versions listed here are considered "safe"; I suppose so long that we're not more than a whole version behind.

Added tests?

• 🙅 no, because they aren't needed

Added to documentation (readme.md or contributing.md)?

• 🙅 no documentation needed

[lpatmo]

Thanks Angelo! Will take a look at merging this in after #4 is merged.

[tgrrr]

I tested this, for starters:

found 0 vulnerabilities

Well done!

What about updating the version of react?

It's still set at:

    "react": "16.11.0",
    "react-dom": "16.11.0",
    "react-scripts": "^3.4.0",

CRA's latest build includes:

    "react": "^16.13.1",
    "react-dom": "^16.13.1",
    "react-scripts": "3.4.3"

After testing a re-install I had a couple of warnings. The usual stuff, but here's the things in our control:

@hapi/joi@17.1.1: joi is leaving the @hapi organization and moving back to 'joi' (https://github.com/sideway/joi/issues/2411)

We can move from @hapi/joi to joi: https://www.npmjs.com/package/joi

eslint-config-semistandard@15.0.1 requires a peer of eslint-config-standard@>=14.1.0 but none is installed. You must install peer dependencies yourself.

We can install eslint-config-standard@>=14.1.0

tsutils@3.17.1 requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.
react-docgen-typescript-loader@3.7.2 requires a peer of typescript@* but none is installed. You must install peer dependencies yourself.
react-docgen-typescript@1.20.4 requires a peer of typescript@>= 3.x but none is installed. You must install peer dependencies yourself.

We can install typescript@>= 3.x

[tgrrr]

What about updating the version of react?

It's still set at:

    "react": "16.11.0",
    "react-dom": "16.11.0",
    "react-scripts": "^3.4.0",

CRA's latest build includes:

    "react": "^16.13.1",
    "react-dom": "^16.13.1",
    "react-scripts": "3.4.3"

[angelocordon]

Thanks @tgrrr - my goal with this PR is to get us up to the safest, stable version of our packages. There were a few cases where we were a good couple of major versions behind. I didn't opt in to upgrade us to the latest versions of packages because in my experience, sometimes latest versions of packages can get us in dependency hell. For example, some months ago, Testing Library was updated but it also required Jest to be at a certain version. As we were using Jest from CRA, we were pinned at their version. In the end, I ended up having to set up Jest manually for this project in order for our testing infrastructure to work cohesively. This is an extreme case, but I'd like to not have to deal with those kinds of problems too often.

As the versions you mentioned are only ~2 minor / patch versions behind, I think we are safe with where we are here.

[tgrrr]

I also had a jest conflict with the current config @angelocordon

[tgrrr]

16.3 is a significant update to react from 16.11 https://github.com/facebook/react/releases

[angelocordon]

@tgrrr re: Jest conflicts - can you expand more on what that conflict is on your end? I'm not seeing any issues on my end other than an async syntax error which I just fixed. Other than that, running npm run test passes all suites 👍🏽. But if you're having something bigger than that, it may be worth filing an issue so we can resolve it outside of this.

re: React updates - the changes from 16.11 to 16.13 don't apply to us, we're not using lazy or memo, nor do we have a multi-root app. We're not dealing with concurrent mode, nor do I see any of the issues from their updates with ReactDOM applying to us. I think we'll eventually get to 16.13 in due time, but as of now, this feels out of the scope of this PR.

[tgrrr]

Storybook requires packages, which throw the following errors:

tsutils@3.17.1 requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.
react-docgen-typescript-loader@3.7.2 requires a peer of typescript@* but none is installed. You must install peer dependencies yourself.
react-docgen-typescript@1.20.4 requires a peer of typescript@>= 3.x but none is installed. You must install peer dependencies yourself.

We can install typescript@>= 3.x

[angelocordon]

Same comment from above; Storybook isn't currently on my priority list as we're not using it in a very effective way, yet.

[tgrrr]

Typescript is also a requirement of material-ui, which this repo heavily depends upon. This is the one library that will provide the most stability if it's added to this PR @angelocordon

[angelocordon]

I'm not seeing anywhere in the Material UI docs that say we need Typescript in order to use Material UI. We've been able to use it this whole time without TS. I think perhaps you mean this page? I believe this only means that if we do have static typing in our codebase, we'd need at least TS 3.2 for it to work with Material.

Considering that this particular thread is about Storybook and we're now talking about Material, this is one of those opportunities where we can be much more practical and avoid some dependency pain points.

[angelocordon]

Thanks for the review @tgrrr - I think the peer dependency issues are... minor 🤷🏽 Curious if you started getting them from upping the React versions?

[lpatmo]

I think this is OK to merge in first, actually, since there are a couple of updates to make in #4!

[tgrrr]

Thanks for the review @tgrrr - I think the peer dependency issues are... minor 🤷🏽 Curious if you started getting them from upping the React versions?

The dependency requirements are based on this PR, without any changes implemented, including to React

npm -v 6.14.5
node -v v12.18.1
MacOS Mojave 10.14.6

With the current PR without any changes I have:

• A bug with Jest and npm, asking me to install from scratch
• The same bug installing with Yarn (not a good sign)
• The missing peer-dependencies.

I understand that shaving the yak is tiresome, but I think there's a large chance of a new dev spending a significant amount of time trying to get this setup.

[angelocordon]

With the current PR without any changes I have:

A bug with Jest and npm, asking me to install from scratch
The same bug installing with Yarn (not a good sign)
The missing peer-dependencies.

Thanks for the heads up @tgrrr. Something feels iffy with your local environment based on the description you're saying. I've removed node_modules and reinstalled the packages, and run the tests and everything seems to be in working order. I think the conflicts you're experiencing may not be related to this PR, but we should file an issue if they're persisting after a deeper investigation.

[angelocordon]

Going ahead to merge this @tgrrr. If any of the discussion topics above need to be resolved further, let's discuss in our frontend channel, or if you continue to have technical issues, please write up an issue.