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

[gatsby-source-contentful] gatsby-plugin-image should be a peerDep #30519

Closed
slk333 opened this issue Mar 28, 2021 · 9 comments · Fixed by #30709
Closed

[gatsby-source-contentful] gatsby-plugin-image should be a peerDep #30519

slk333 opened this issue Mar 28, 2021 · 9 comments · Fixed by #30709
Labels
topic: source-contentful Related to Gatsby's integration with Contentful type: bug An issue or pull request relating to a bug in Gatsby

Comments

@slk333
Copy link
Contributor

slk333 commented Mar 28, 2021

Hello!

On this documentation page:
https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-source-contentful#using-the-new-gatsby-image-plugin

The documentation currently states:

  1. Install the plugins:
    npm install gatsby-plugin-image gatsby-plugin-sharp

But gatsby-plugin-image is already a dependency of gatsby-source-contentful, and it works fine without the redundant install.

I didn't open a Pull Request because I'm not sure if this is intended or not.

Thanks

@slk333 slk333 added the type: documentation An issue or pull request for improving or updating Gatsby's documentation label Mar 28, 2021
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 28, 2021
@LekoArts LekoArts removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Apr 6, 2021
@LekoArts
Copy link
Contributor

LekoArts commented Apr 6, 2021

Hey, thanks for the issue! The docs are correct here, gatsby-plugin-image should be installed as it really should be a peerDependency not a direct dependency.

@LekoArts LekoArts added topic: source-contentful Related to Gatsby's integration with Contentful type: bug An issue or pull request relating to a bug in Gatsby and removed type: documentation An issue or pull request for improving or updating Gatsby's documentation labels Apr 6, 2021
@LekoArts LekoArts changed the title [gatsby-source-contentful] redundant dependency [gatsby-source-contentful] gatsby-plugin-image should be a peerDep Apr 6, 2021
@axe312ger
Copy link
Collaborator

Good find, thats outdated. Readme should not contain the install for gatsby-plugin-sharp

@axe312ger
Copy link
Collaborator

@LekoArts no, we need it as a dependency in the plugin.

@LekoArts
Copy link
Contributor

LekoArts commented Apr 6, 2021

Why is that? @ascorbic documented it differently: https://www.gatsbyjs.com/docs/how-to/plugins-and-themes/adding-gatsby-image-support/#other-considerations

gatsby-plugin-sharp is a peerDep

@axe312ger
Copy link
Collaborator

It was required when implementing gatsby-plugin-image. Now it is not anymore ¯_(ツ)_/¯

So yeah, can become a peer dep. Checked the code, all requires should check if the plugin is actually installed

@ascorbic
Copy link
Contributor

ascorbic commented Apr 6, 2021

@axe312ger The docs are correct: it should be a peerDependency and a devDependency.

@axe312ger
Copy link
Collaborator

@ascorbic both? 🤔

@ascorbic
Copy link
Contributor

ascorbic commented Apr 6, 2021

You can probably skip the devDependency, as you're in the monorepo, but it's for whiel you're developing.

@axe312ger
Copy link
Collaborator

PR incoming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-contentful Related to Gatsby's integration with Contentful type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants