react-uswds icon indicating copy to clipboard operation
react-uswds copied to clipboard

yarn is required to install this library with npm

Open NatHillardUSDS opened this issue 4 years ago • 7 comments

ReactUSWDS Version & USWDS Version: 1.17.0 , USWDS bundled with this

Describe the bug We recently ran into an issue on a clean install using npm install - full log attached, and relevant excerpt below:

4038 verbose prepareGitDep @trussworks/react-uswds@github:nathillardusds/react-uswds#nathillardusds/ssr: installing devDeps and running prepare script.
4039 error prepareGitDep 1>
4039 error prepareGitDep > [email protected] install /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/watchpack-chokidar2/node_modules/fsevents
4039 error prepareGitDep > node install.js
4039 error prepareGitDep
4039 error prepareGitDep   SOLINK_MODULE(target) Release/.node
4039 error prepareGitDep   CXX(target) Release/obj.target/fse/fsevents.o
4039 error prepareGitDep   SOLINK_MODULE(target) Release/fse.node
4039 error prepareGitDep
4039 error prepareGitDep > [email protected] install /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/husky
4039 error prepareGitDep > node husky install
4039 error prepareGitDep
4039 error prepareGitDep husky > Setting up git hooks
4039 error prepareGitDep husky > Done
4039 error prepareGitDep
4039 error prepareGitDep > [email protected] postinstall /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/@babel/polyfill/node_modules/core-js
4039 error prepareGitDep > node -e "try{require('./postinstall')}catch(e){}"
4039 error prepareGitDep
4039 error prepareGitDep [96mThank you for using core-js ([94m https://github.com/zloirock/core-js [96m) for polyfilling JavaScript standard library![0m
4039 error prepareGitDep
4039 error prepareGitDep [96mThe project needs your help! Please consider supporting of core-js on Open Collective or Patreon: [0m
4039 error prepareGitDep [96m>[94m https://opencollective.com/core-js [0m
4039 error prepareGitDep [96m>[94m https://www.patreon.com/zloirock [0m
4039 error prepareGitDep
4039 error prepareGitDep [96mAlso, the author of core-js ([94m https://github.com/zloirock [96m) is looking for a good job -)[0m
4039 error prepareGitDep
4039 error prepareGitDep
4039 error prepareGitDep > [email protected] postinstall /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/core-js
4039 error prepareGitDep > node -e "try{require('./postinstall')}catch(e){}"
4039 error prepareGitDep
4039 error prepareGitDep
4039 error prepareGitDep > [email protected] postinstall /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/core-js-pure
4039 error prepareGitDep > node -e "try{require('./postinstall')}catch(e){}"
4039 error prepareGitDep
4039 error prepareGitDep
4039 error prepareGitDep > [email protected] postinstall /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99/node_modules/husky
4039 error prepareGitDep > opencollective-postinstall || exit 0
4039 error prepareGitDep
4039 error prepareGitDep [96m[1mThank you for using husky![96m[1m
4039 error prepareGitDep [0m[96mIf you rely on this package, please consider supporting our open collective:[22m[39m
4039 error prepareGitDep > [94mhttps://opencollective.com/husky/donate[0m
4039 error prepareGitDep
4039 error prepareGitDep
4039 error prepareGitDep > @trussworks/[email protected] prepare /Users/nathillard/.npm/_cacache/tmp/git-clone-72e0de99
4039 error prepareGitDep > yarn build
4040 error prepareGitDep 2> npm WARN install Usage of the `--dev` option is deprecated. Use `--also=dev` instead.
4040 error prepareGitDep npm WARN deprecated @types/[email protected]: This is a stub types definition. classnames provides its own type definitions, so you do not need this installed.
4040 error prepareGitDep npm WARN deprecated [email protected]: babel-eslint is now @babel/eslint-parser. This package will no longer receive updates.
4040 error prepareGitDep npm WARN deprecated @babel/[email protected]: 🚨 This package has been deprecated in favor of separate inclusion of a polyfill and regenerator-runtime (when needed). See the @babel/polyfill docs (https://babeljs.io/docs/en/babel-polyfill) for more information.
4040 error prepareGitDep npm WARN deprecated [email protected]: The gitlab package has found a new home in the @gitbeaker organization. For the latest gitlab node library, check out @gitbeaker/node. A full list of the features can be found here: https://github.com/jdalrymple/gitbeaker#readme
4040 error prepareGitDep npm WARN deprecated [email protected]: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142
4040 error prepareGitDep npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
4040 error prepareGitDep npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
4040 error prepareGitDep npm WARN deprecated [email protected]: core-js@<3.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Please, upgrade your dependencies to the actual version of core-js.
4040 error prepareGitDep npm WARN deprecated [email protected]: this library is no longer supported
4040 error prepareGitDep npm WARN deprecated [email protected]: some dependency vulnerabilities fixed, support for node < 10 dropped, and newer ECMAScript syntax/features added
4040 error prepareGitDep npm WARN deprecated [email protected]: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
4040 error prepareGitDep npm WARN deprecated [email protected]: https://github.com/lydell/resolve-url#deprecated
4040 error prepareGitDep npm WARN deprecated [email protected]: Please see https://github.com/lydell/urix#deprecated
4040 error prepareGitDep npm WARN deprecated [email protected]: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
4040 error prepareGitDep sh: yarn: command not found
4040 error prepareGitDep npm ERR! code ELIFECYCLE
4040 error prepareGitDep npm ERR! syscall spawn
4040 error prepareGitDep npm ERR! file sh
4040 error prepareGitDep npm ERR! errno ENOENT
4040 error prepareGitDep npm ERR! @trussworks/[email protected] prepare: `yarn build`
4040 error prepareGitDep npm ERR! spawn ENOENT
4040 error prepareGitDep npm ERR!
4040 error prepareGitDep npm ERR! Failed at the @trussworks/[email protected] prepare script.

(this was on a fork of this repo pointed to by a github:-prefixed path, but it appears the issue exists in the main repo as well)

It appears the issue is in this line: https://github.com/trussworks/react-uswds/blob/41a1cdc2e3fb96e7db2f7f4f05145798485ce7c6/package.json#L34

which assumes the use of yarn -- we are using npm for our project, and while its a reasonable assumption that folks would have installed yarn, this particular machine didn't have yarn installed and thus encountered this. If we do npm install -g yarn it goes away.

I believe the solution may be to use npm build in the prepare script instead, but I'm not sure how this is typically handled, so there may be a better way ?

To Reproduce

  1. On clean OS install
  2. clone this project
  3. run npm install with node v. 14.17.1 (installed via asdf but this shouldn't make a difference), and without yarn installed.
  4. See error above

Expected behavior

Should be able to use the npm install toolchain without needing yarn installed

Screenshots N/A

Additional context Full install log: uswds.log

Device and Browser Information (please complete the following information if describing a UI bug): macOS Big Sur 11.4 (20F71) - M1 Apple Silicon - node v14.17.1 installed via arch -x86_64 asdf install nodejs 14.17.1

(Updated to use template)

NatHillardUSDS avatar Jun 28 '21 21:06 NatHillardUSDS

The https://github.com/trussworks/react-uswds/blob/main/docs/contributing.md specifies yarn, which seems fine for building or modifying the library itself. Yarn is pretty common on React projects since create-react-app uses it. I don't know of a lot of projects that can build with either, especially since the two use different lockfiles. Yarn issues a warning if it finds a package-lock.json file since it uses yarn.lock itself.

If react-uswds used as a dependency in a project you should be able to use either npm or yarn there.

dmethvin-gov avatar Jul 16 '21 13:07 dmethvin-gov

(Hi Dave!) - Also though, and apologies for only just getting back to this, and for the initial lack of clarity. The issue is not that react-uswds uses yarn, but rather, we are seeing failures in our own project (henceforth "J40"), which has react-uswds as a dependency and which, itself, only uses npm, when we run an npm build. To your point, If react-uswds used as a dependency in a project you should be able to use either npm or yarn there: it should be possible to install this on a machine without yarn installed, this appears to be the bug.

To repro:

  1. Clone our project, https://github.com/usds/justice40-tool/, on a fresh machine (henceforth J40). cd justice40-tool.
  2. Run npm install. Note that J40 has react-uswds as a dependency
  3. Encounter the above logs / the yarn: command not found error, which is a side effect of prepare cited above, because I don't have yarn installed globally
  4. npm install -g yarn gets around this, but seems odd that I would have to install yarn just in order to install my dependency

Looking for precedent around this, I found this issue, though that being react-native it seems like a slightly different scenario.

There's also this which suggests using yarn-or-npm to work around this. This seems to have a fair number of users and seems to have been designed around this use case. There are also several projects using npm_execpath to test availability.

In any case, we have not seen this with other projects, even those like react itself which uses yarn (see [their https://github.com/facebook/react/blob/main/package.json ) , and there appears to be solutions like the above to this problem, so we thought to call it out. Overall minor, in that it is possible to install yarn on these machines, but seems there are ways around this that other projects have pursued

NatHillardUSDS avatar Aug 18 '21 04:08 NatHillardUSDS

Your initial report was clear enough, I just didn't understand it thoroughly. 🤦 It's unusual for a library to do a build on the dependent system, at least in my experience. The most typical thing for libraries to do is to build and publish the artifacts on npm, for example in a dist directory (that is .gitignoreed), rather than having the consuming project do that. For one, it's safer since a library doesn't know the consumer's environment. The lack of yarn is just one issue so fixing that won't really solve all of it.

Bottom line, I'd say this is a valid issue.

dmethvin-gov avatar Aug 18 '21 14:08 dmethvin-gov

The most typical thing for libraries to do is to build and publish the artifacts on npm, for example in a dist directory (that is .gitignoreed), rather than having the consuming project do that.

This is our desired approach. From the original post:

(this was on a fork of this repo pointed to by a github:-prefixed path, but it appears the issue exists in the main repo as well)

Since the fork is not pointing to a published artifact, I imagine this is the source of the issue. We can look into supporting this case, but as of right now, the published artifacts should be used, otherwise yarn is required to build the project (directly or transitively, it seems).

@NatHillardUSDS Is the project fork under your control? If so, can you update the package.json in the fork to suit your needs as a workaround if the fork must be used?

Does the fork exist to cover some gap in functionality that this project is currently missing? Perhaps in that case the better long-term solution would be to close that gap and make sure a published npm artifact exists so that the github: url isn't needed.

Any more context around this case would be great to see what level we are able to support this, or what the best solution is 😄.

brandonlenz avatar Sep 20 '21 16:09 brandonlenz

Hi Brandon - Thank you for the response! This makes sense, and I had overlooked that this might be the reason why it was attempting to build.

As a bit more context, the fork was originally created to work around this issue - that said, we have since moved away from directly using FooterNav, and are now using the repo directly, rather than the fork.

After moving back to the standard repo / away from our fork, it now works again to run npm i without having yarn installed, and the reasoning you give for this makes sense. It should be OK to close this current issue given this explanation. Thanks again for taking the time to look into this!

NatHillardUSDS avatar Sep 20 '21 17:09 NatHillardUSDS

I looked into this a few weeks ago but forgot to create a PR for it. I just made #1583 to document what I found. In general that PR can kinda-sorta make npm work for installs from a git commit, but there are some drawbacks that might argue against it. In any case feel free to close this issue and the PR if it seems better to leave things as-is.

dmethvin avatar Sep 21 '21 00:09 dmethvin

@NatHillardUSDS thanks for that additional context! Some maintainers had a conversation about support for issues like https://github.com/trussworks/react-uswds/issues/1488, and it seems we're in agreement: we want to make sure this library is usable for server-side development, so we hope to resolve that issue directly, and come up with some safeguards for introducing code that leads to those types of bugs in the first place 😃

Thanks @dmethvin for making that PR! That's super helpful. We'll get some eyes on it and keep the conversation going!

brandonlenz avatar Sep 23 '21 15:09 brandonlenz