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

Some gatsby package dependencies should be devDependencies #20815

Closed
DevSide opened this issue Jan 23, 2020 · 7 comments
Closed

Some gatsby package dependencies should be devDependencies #20815

DevSide opened this issue Jan 23, 2020 · 7 comments
Assignees
Labels
good first issue Issue that doesn't require previous experience with Gatsby type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change

Comments

@DevSide
Copy link
Contributor

DevSide commented Jan 23, 2020

Description

Migrate some dependencies in devDependencies for gatsby package.

At least these (non exhaustive):

  • "@typescript-eslint/eslint-plugin"
  • "@typescript-eslint/parser"
  • "eslint"
  • "eslint-config-react-app"
  • "eslint-loader"
  • "eslint-plugin-flowtype"
  • "eslint-plugin-graphql"
  • "eslint-plugin-import"
  • "eslint-plugin-jsx-a11y"
  • "eslint-plugin-react"
  • "eslint-plugin-react-hooks"

Steps to reproduce

yarn install --production
npm install --production

Expected result

No dev packages should be installed.

Actual result

Load linting dependencies and others

Environment

All.

@sidharthachatterjee
Copy link
Contributor

sidharthachatterjee commented Jan 23, 2020

"eslint"
"eslint-config-react-app"
"eslint-loader"
"eslint-plugin-graphql"
"eslint-plugin-import"
"eslint-plugin-jsx-a11y"
"eslint-plugin-react"
"eslint-plugin-react-hooks"

These are correctly set to dependencies because they're used when running gatsby develop (Gatsby lints your site's code)

"eslint-plugin-flowtype"
"@typescript-eslint/eslint-plugin"
"@typescript-eslint/parser"

These on the other hand are indeed devDependencies and could be moved there.

@sidharthachatterjee sidharthachatterjee added good first issue Issue that doesn't require previous experience with Gatsby type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change labels Jan 23, 2020
@maecapozzi
Copy link
Contributor

I can take this.

@sidharthachatterjee
Copy link
Contributor

Flow and TypeScript packages should even be optionalDependencies.

Hmm, after some thinking I just realised that these shouldn't be dependencies of gatsby but in our monorepo root package.json since they're used when developing gatsby (and not a gatsby site)

@maecapozzi
Copy link
Contributor

Flow and TypeScript packages should even be optionalDependencies.

Hmm, after some thinking I just realised that these shouldn't be dependencies of gatsby but in our monorepo root package.json since they're used when developing gatsby (and not a gatsby site)

Just to make sure I understand, I can remove all dependencies related to typescript and flow from /packages/gatsby?

So for example, I can ditch eslint-plugin-flowtype.

@maecapozzi
Copy link
Contributor

maecapozzi commented Jan 29, 2020

@sidharthachatterjee @pieh I'm following up because I'm losing the thread a bit here between the issue and the PR.

Would one of you let me know what you'd like?

@LekoArts
Copy link
Contributor

cc @sidharthachatterjee @pieh

@pieh
Copy link
Contributor

pieh commented Mar 12, 2020

Can't remove those deps or move to dev deps because of #20815 (comment) - the additional 3 deps there are peer deps of eslint-config-react-app so they also need to stay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with Gatsby type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants