Skip to content
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

docs(example-app): add create-react-app example app #206

Merged
merged 8 commits into from
Jun 1, 2020

Conversation

suzubara
Copy link
Contributor

@suzubara suzubara commented May 26, 2020

Fixes #77

  • Set up create-react-app boilerplate in /example using the Typescript template
  • Add @trussworks/react-uswds as a local dependency
  • Fix example app React module resolution
  • Render GovBanner in App to demo the lib is working correctly
➜ yarn audit
yarn audit v1.13.0
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > @commitlint/lint > @commitlint/parse >     │
│               │ conventional-commits-parser > meow > yargs-parser            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump >           │
│               │ conventional-commits-parser > meow > yargs-parser            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > @commitlint/read > git-raw-commits > meow  │
│               │ > yargs-parser                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ marked                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.6.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-info                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-info > marksy > marked                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/812                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > meow > yargs-parser                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump >           │
│               │ git-semver-tags > meow > yargs-parser                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > git-semver-tags > meow > yargs-parser     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump > meow >    │
│               │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
8 vulnerabilities found - Packages audited: 2231
Severity: 7 Low | 1 Moderate
✨  Done in 1.91s.

@suzubara suzubara added status: wip Work in progress - not ready for code review type: example Example implementations of ReactUSWDS, such as Storybook or kitchen sink app labels May 26, 2020
@trussworks-infra-zz
Copy link

trussworks-infra-zz commented May 27, 2020

Warnings
⚠️ This PR does not include changes to storybook, even though it affects component code.

Generated by 🚫 dangerJS against 6546c36

@suzubara
Copy link
Contributor Author

I wanted to note that in this PR I explicitly tell Jest to ignore /example when running yarn test at the root. The question of whether the library tests should also run example app tests is up in the air, as well as how they should factor into CI. I made an issue here to discuss that: #208

@suzubara suzubara marked this pull request as ready for review May 27, 2020 22:14
@suzubara suzubara removed the status: wip Work in progress - not ready for code review label May 28, 2020
Copy link
Contributor

@haworku haworku left a comment

Choose a reason for hiding this comment

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

Question - Should we also .npmignore the example folder?

Also, I'm seeing an issue with babel-jest and react-scripts that's keeping me from running the yarn scripts. Did you run into this? It seems like having babel-jest somewhere outside the project directory (in this case in the parent) but also brought in by react-scripts makes things not happy.

const path = require('path')

// This is needed in order to correctly resolve React versions between the library & example app
module.exports = override(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to be needed for any other (eventual) shared dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't, as far as I'm aware react notably causes issues if multiple instances are loaded and that's what this workaround is for.

@suzubara
Copy link
Contributor Author

Question - Should we also .npmignore the example folder?

ok I just reminded myself of this: we don't need to use .npmignore because we are defining the files array in package.json which whitelists files to include in the package instead of the other way around. from https://docs.npmjs.com/misc/developers:

If, given the structure of your project, you find .npmignore to be a maintenance headache, you might instead try populating the files property of package.json, which is an array of file or directory names that should be included in your package. Sometimes a whitelist is easier to manage than a blacklist.

@suzubara
Copy link
Contributor Author

Also, I'm seeing an issue with babel-jest and react-scripts that's keeping me from running the yarn scripts. Did you run into this? It seems like having babel-jest somewhere outside the project directory (in this case in the parent) but also brought in by react-scripts makes things not happy.

update: added SKIP_PREFLIGHT_CHECK=true to the yarn scripts as well as a note about it in the readme

@suzubara suzubara requested a review from haworku May 29, 2020 20:49
Copy link
Contributor

@eamahanna eamahanna left a comment

Choose a reason for hiding this comment

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

LGTM

@suzubara suzubara changed the title feat(example-app): add create-react-app example app docs(example-app): add create-react-app example app Jun 1, 2020
@suzubara suzubara merged commit cf28086 into develop Jun 1, 2020
@suzubara suzubara deleted the sr-example-app-77 branch June 1, 2020 17:21
@suzubara suzubara mentioned this pull request Jun 3, 2020
@suzubara suzubara mentioned this pull request Jun 3, 2020
@haworku haworku mentioned this pull request Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: example Example implementations of ReactUSWDS, such as Storybook or kitchen sink app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add example application
4 participants