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

fix: add missing dependencies #31837

Merged
merged 5 commits into from
Sep 6, 2021

Conversation

merceyz
Copy link
Contributor

@merceyz merceyz commented Jun 8, 2021

Description

I'm removing the old dependency fallback to gatsby from the PnP runtime since it causes hard to debug issues and hides undeclared dependencies, this PR adds the missing dependencies uncovered by the changes.

Related Issues

yarnpkg/berry#3004

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 8, 2021
@merceyz merceyz changed the title fix(cli): add missing dependencies fix: add missing dependencies Jun 8, 2021
@merceyz merceyz force-pushed the merceyz/fix/cli-missing-deps branch 2 times, most recently from c1ab4d0 to eae29b1 Compare June 11, 2021 10:08
@merceyz
Copy link
Contributor Author

merceyz commented Jun 11, 2021

Any suggestions for how to fix the failing CI checks?

@TylerBarnes TylerBarnes added status: needs core review Currently awaiting review from Core team member and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jun 11, 2021
@TylerBarnes
Copy link
Contributor

@merceyz try merging master into your branch

@merceyz merceyz force-pushed the merceyz/fix/cli-missing-deps branch from 924ccef to b9a3896 Compare June 11, 2021 22:59
@merceyz
Copy link
Contributor Author

merceyz commented Jun 11, 2021

I've done that a few times already

@merceyz merceyz force-pushed the merceyz/fix/cli-missing-deps branch from b9a3896 to 6378692 Compare June 29, 2021 13:49
@wardpeet wardpeet force-pushed the merceyz/fix/cli-missing-deps branch from 797b712 to 43f69e5 Compare September 2, 2021 21:28
@wardpeet wardpeet marked this pull request as ready for review September 6, 2021 12:31
@wardpeet wardpeet added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change bot: merge on green Gatsbot will merge these PRs automatically when all tests passes labels Sep 6, 2021
@@ -30,8 +30,7 @@
"react-dom": "^17.0.1",
"react-helmet": "^6.1.0",
"typeface-merriweather": "0.0.72",
"typeface-montserrat": "0.0.75",
"webpack": "^5.35.0"
Copy link
Contributor Author

@merceyz merceyz Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wardpeet I added this because some of the gatsby plugins requires webpack to be provided by the parent, removing it will make them unmet peer dependencies and be rejected by PnP again and cause it to rely on hoisting under node_modules

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting cause we don't want that people install webpack, i'll see why it's necessary here.

@gatsbybot gatsbybot merged commit ec453f2 into gatsbyjs:master Sep 6, 2021
@merceyz merceyz deleted the merceyz/fix/cli-missing-deps branch September 6, 2021 13:40
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
* chore(starters): avoid using npm in scripts

* ci: test with no fallbacks

* fix: add missing dependencies

* update

Co-authored-by: Ward Peeters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes status: needs core review Currently awaiting review from Core team member 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 this pull request may close these issues.

4 participants