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

Consider using peerDependencies for including react, prop-types, etc. #1990

Closed
kripod opened this issue Sep 1, 2017 · 10 comments
Closed

Consider using peerDependencies for including react, prop-types, etc. #1990

kripod opened this issue Sep 1, 2017 · 10 comments
Assignees

Comments

@kripod
Copy link
Contributor

kripod commented Sep 1, 2017

Using ESLint along with Gatsby, I encountered several violations of the ESLint rule import/no-extraneous-dependencies. As a workaround, @ahfarmer has once suggested to use the ESLint config snippet below:

  ...
  settings: {
    // These packages are provided 'magically' by Gatsby
    'import/core-modules': ['react', 'config'],
  },
  rules: {
  ...

Personally, I find it misleading that some source-bound packages are not listed in package.json, and would like to suggest requiring packages similar to react by specifying them as peerDependencies of their corresponding Gatsby plugins.

@kripod
Copy link
Contributor Author

kripod commented Sep 3, 2017

Things also become inconsistent when using glamorous along with gatsby-plugin-glamor: yarn always warns me to install glamor, as it’s a peerDependency of glamorous, but a specific version of glamor is already included in its corresponding Gatsby plugin.

@danoc
Copy link
Contributor

danoc commented Nov 27, 2017

This would be great for cleaning up incorrect "unmet peer dependency" warnings.

Also, it seems inline with the approach taken in the recent Gatsby breaking changes for Styled Components and React Helmet.

@flipactual
Copy link
Contributor

I can take this on if it's up for grabs from non-Contributors!

@flipactual flipactual self-assigned this Jan 20, 2018
@flipactual
Copy link
Contributor

I'm trying to discern the desired workflow here

Let's say you're using gatsby-plugin-glamor and you'd also like to have glamor in your gatsby project itself, cool, we can handle that with peer deps, but we have to change how the plugin is installed for everyone

Would you like the documentation to say that to install you should run npm install --save gatsby-plugin-glamor glamor@^2.20.29?

I believe (I'd be thrilled to be mistaken on this) that we need to specify the version of the peer dependency that we're installing in order to keep things in sync – I don't believe there's manually resolution where it's like hey cool there's a peer dep in this first thing so let's just grab that version of this second thing

Can we/do we want to handle this automatically in a post install?

Do we want to handle this like eslint-config-airbnb and provide a guide for getting the version numbers?

Do we want to use https://github.com/emdaer/emdaer (shameless self-promotion) to keep the peer dep versions up to date in the README install steps by pulling them from package.json?

Or am I missing another even better option?

@KyleAMathews
Copy link
Contributor

@flipactual check out the plugins we've already converted like styled-components and emotion — https://www.gatsbyjs.org/packages/gatsby-plugin-styled-components/

We just tell people to install both the plugin and the actual package.

I think it's fine to tell people just to install the latest version of the peerDependency — yeah there's the risk of there being a major release which breaks things but that's rare and quickly fixable generally.

Generally speaking I dislike extra process and work unless it's proven it's needed.

@flipactual
Copy link
Contributor

Cool cool cool

What all from gatsby should be moved to peerDependencies? I know react, probably react-dev-utils, redux?

@KyleAMathews
Copy link
Contributor

Just react and react-dom. The point of a peerDependency is that it allows a user to easily upgrade to newer versions without us Gatsby needing to do anything. So when React 17 comes out, people can upgrade to React 17 without us having to release a new major version of Gatsby.

redux is an internal dependency so not relevant to users. If they want to use Redux for their site, they'll install their own version.

@KyleAMathews
Copy link
Contributor

I think this is done now for v2!

@robwierzbowski
Copy link

Should Webpack be listed as a peer dep? Working on an existing project using Gatsby, and seeing that webpack is available in gatsby-node, although it's not in the projects devDeps.

@Pustur
Copy link

Pustur commented Oct 1, 2018

Sorry to bring this up again, I'm still having this issue with the package prop-types in a new gatsby v2 installation using the default starter.

Should i just get rid of it using npm i -S prop-types? Or is this supposed to be in package.json from the beginning?

(Maybe this is an issue with the starter itself?)

EDIT: Seems like it was a bug with the default starter, it's reported here: https://github.com/gatsbyjs/gatsby-starter-default/issues/94

Sorry for the unnecessary noise.

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

No branches or pull requests

7 participants