Install us-form-system peerDependencies as devDependencies

[annekainicUSDS]

This PR does 2 things:

1. Update version of us-forms-system
2. Install us-forms-system's peerDependencies as devDependencies here.

[dmethvin-gov]

I tested with the react and formation reps moved to dependencies, and without the uswds dependency, and it seemed to build/run fine. here.

[dmethvin-gov]

I think this should be a dependency? It's referenced in

us-forms-system-starter-app/js/components/Introduction.jsx

Line 2 in 9e1247b

import ProgressButton from '@department-of-veterans-affairs/formation/ProgressButton';

so it's needed at runtime, thus a dependency and not devDependency.

[annekainicUSDS]

@dmethvin-gov I think react can remain a devDependency? In vets-website we install it as a devDependency, as does create-react-app. I believe this is because we're using webpack to create a static bundle as part of the build step, but I'm not 100% positive. Still trying to understand this better.

[dmethvin-gov]

Same with these two, I think they're dependencies.

[dmethvin-gov]

I don't see uswds referenced at all in the starter app so I don't think it needs to be here. It gets pulled in indirectly via us-form-system.

[annekainicUSDS]

Hm I'm not able to run the app without having uswds installed locally; styles.css from us-forms-system references images that come from the node module for uswds. If I run npm start locally on this app I get errors. I already have a separate ticket to look into the best way to import uswds images (usds/us-forms-system#66), but for now I think we still need to install uswds as a devDependency. If you have suggestions for the images problem please feel free to comment in that other ticket!

[dmethvin-gov]

I'm not seeing any image errors when I pull the current HEAD, remove node_modules, move the two react dependencies, npm install, and npm start. I do see that it's unable to find the fonts.

[annekainicUSDS]

I think you need to pull this branch in order to see the issue, not HEAD.

[dmethvin-gov]

OK, gotcha. Let me look at that, the app definitely shouldn't be the middleman here but I'm not sure how to avoid it at the moment. I'll comment over on that ticket.

[annekainicUSDS]

@dmethvin-gov do you mind taking another look at this? I reinstalled uswds because for me I was getting build errors without it when I tried to run npm start (see commit b36f85d here). I also added a comment in regards to react being a devDependency or not; I'm still a little confused on this myself so happy to have a discussion about that!

[dmethvin-gov]

When I pull this in and rebuild I don't get any errors, which is good. The USWDS dependency still seems strange to me.

The way I've always understood dependencies is that if a project needs something, directly, at runtime (for example React) then it should be in dependencies. If it's only needed during project build (for example Babel) it should be in devDependencies. If you're writing something that could be considered a "plugin" for another package, where you assume the parent (including) app will pull in the package, then it's peerDependencies.

So if USWDS is a peer dependency in us-forms-system it would seem like we should use uswds directly in the app itself. Perhaps the starter is a degenerative case since we're using the forms library in a skeletal form. Or maybe it's a sign we need to refactor some of these parts; in a real app it's likely that the developer would want to modify the default USWDS styles which means they'd either need to have a modified USWDS package or they would need to override the USWDS styles and fonts in some additional CSS.

In any case let's land this for now and I'll continue to get to know how this works.

[annekainicUSDS]

@dmethvin-gov I think you're totally right. Ideally, we wouldn't need USWDS installed if we're not calling it directly from within this app. I think it has to do with how I've imported the css and images, which I know we have separate tickets for to refactor. So I will merge this now and hopefully we will improve this through the other tickets.