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

Octogonz/add peer deps #1

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

octogonz
Copy link

@octogonz octogonz commented Dec 31, 2020

Fix errors printed by the ESLint extension for VS Code:

  1. my-app's .eslintrc.json said "extends": ["react-app"] which is an undeclared dependency on the eslint-config-react-app package (called a "phantom dependency"). I replaced the .eslintrc.json shorthand with the fully qualified name to make this more obvious.
  2. The installation model used by NPM and Yarn tolerates phantom dependencies (and even encourages this practice, referring to it as "hoisting"). Whereas phantom dependencies generally will not work with the installation models of PNPM, Rush, and Yarn-PNP -- this is a feature, not a bug: Phantom dependencies cause confusing problems for nontrivial versioning relationships, as commonly arise in a large monorepo. Thus we need to add "eslint-config-react-app" to your package.json file.
  3. eslint-config-react-app has a bunch of peer dependencies on other ESLint packages. They are declared as peers because of a longstanding issue where ESLint looks for dependencies in the my-app folder instead of in the eslint-config-react-app folder.
  4. These peer dependencies need to be declared in your my-app/package.json file. If you don't do that, it will work with NPM because of a controversial behavior where NPM silently installs peer dependencies. But it generally won't work with PNPM, Rush, or Yarn-PNP. (In fact Rush and PNPM have a recommended setting strictPeerDependencies that makes this an error rather than a warning.)

These issues all reflect that create-react-app is designed for starter projects and assumes that you are using NPM's installation model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant