Skip to content

Update gatsby package through yarn upgrade-interactive #222

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

Merged
merged 2 commits into from
Nov 2, 2017

Conversation

efallancy
Copy link
Contributor

This PR is meant to upgrade GatsbyJS to be able to get the latest update for gatsby-react-router-scroll that result into error in ScrollContext and causes the component to be unmounted. This will be able to resolve issue in #14

@bvaughn , I hope this helps :)

@reactjs-bot
Copy link

reactjs-bot commented Oct 30, 2017

Deploy preview ready!

Built with commit f9d2e2a

https://deploy-preview-222--reactjs.netlify.com

@bvaughn
Copy link
Contributor

bvaughn commented Oct 30, 2017

Can confirm this PR fixes the issue originally reported against Firefox and the cookies whitelist plug-in.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Unfortunately it looks like Gatsby fails on a fresh install and build.

rm -rf .cache
rm -rf node_modules
yarn install
yarn build

screen shot 2017-10-30 at 12 54 54 pm

This error does not reproduce on master.

I've seen this happen before when upgrading Gatsby and/or plug-ins. Unfortunately the only way to track it down is to upgrade things incrementally until you identify the package/version that broken.

@efallancy
Copy link
Contributor Author

@bvaughn yikes! interesting... I tried running the same command that you did like below:

rm -rf .cache
rm -rf node_modules
yarn install
yarn build

Somehow, I can run the build without any error 🤔 . I'll check further and update you on it 😄 Let me know if I miss anything from the command or any other situation that could lead to that error

@KyleAMathews
Copy link
Contributor

I tried upgrading as well and it worked just fine. Perhaps install fail? Also that looks like our old duplicate GraphQLs friend :-|

Also FYI, I'm working soon on new testing infrastructure for Gatsby that'll build all example sites + a bunch of community sites including reactjs.org on every PR so we'll be making sure reactjs.org is always upgradable :-)

@efallancy
Copy link
Contributor Author

@bvaughn I've tried the same steps that you mentioned before and there's no error happening on my side. I am sure @KyleAMathews would have a great explanation to this. I can't reproduce the error at all 😐

@bvaughn
Copy link
Contributor

bvaughn commented Oct 31, 2017

Also FYI, I'm working soon on new testing infrastructure for Gatsby that'll build all example sites + a bunch of community sites including reactjs.org on every PR so we'll be making sure reactjs.org is always upgradable :-)

This is fantastic! I was thinking about this over the weekend, wishing we had infrastructure like that setup. 👏 Nice!

@bvaughn I've tried the same steps that you mentioned before and there's no error happening on my side.

I will give it another try today!

@bvaughn
Copy link
Contributor

bvaughn commented Nov 1, 2017

Failed again for me locally. (Same error as above.)

Looking in yarn.lock I see:

[email protected], graphql@^0.10.3, graphql@^0.10.5:
  version "0.10.5"
  resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.10.5.tgz#c9be17ca2bdfdbd134077ffd9bbaa48b8becd298"
  dependencies:
    iterall "^1.1.0"

graphql@^0.11.3:
  version "0.11.7"
  resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.11.7.tgz#e5abaa9cb7b7cccb84e9f0836bf4370d268750c6"
  dependencies:
    iterall "1.1.3"

And looking further for why this is, I see:

[email protected]:
  dependencies:
    graphql "^0.10.5"

eslint-plugin-relay@^0.0.19:
  dependencies:
    graphql "^0.11.3"

gatsby@^1.9.9:
  dependencies:
    graphql "^0.10.3"

So the fix for me is to add another "resolution" to package.json:

  "eslint-plugin-relay/graphql": "0.10.5"

It's interesting though that you two aren't seeing this behavior on a fresh install. Which Yarn version are you both using? (1.2.0 here.)

@bvaughn
Copy link
Contributor

bvaughn commented Nov 1, 2017

In the meanwhile, @emmafallancy, would you be willing to update this PR with the following change?

diff --git a/package.json b/package.json
index dbb60bb28..9adc56da2 100644
--- a/package.json
+++ b/package.json
@@ -9,7 +9,8 @@
   "resolutions": {
     "gatsby/graphql": "0.10.5",
     "gatsby/react": "16.0.0",
-    "gatsby/react-dom": "16.0.0"
+    "gatsby/react-dom": "16.0.0",
+    "eslint-plugin-relay/graphql": "0.10.5"
   },
   "dependencies": {
     "@gaearon/react-live": "1.7.1-with-safari-fix",
diff --git a/yarn.lock b/yarn.lock
index 466fa8040..a7cd0524b 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -4485,18 +4485,12 @@ graphql-skip-limit@^1.0.6:
   dependencies:
     babel-runtime "^6.26.0"
 
-[email protected], graphql@^0.10.3, graphql@^0.10.5:
+[email protected], graphql@^0.10.3, graphql@^0.10.5, graphql@^0.11.3:
   version "0.10.5"
   resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.10.5.tgz#c9be17ca2bdfdbd134077ffd9bbaa48b8becd298"
   dependencies:
     iterall "^1.1.0"
 
-graphql@^0.11.3:
-  version "0.11.7"
-  resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.11.7.tgz#e5abaa9cb7b7cccb84e9f0836bf4370d268750c6"
-  dependencies:
-    iterall "1.1.3"
-
 gray-matter@^2.1.0:
   version "2.1.1"
   resolved "https://registry.yarnpkg.com/gray-matter/-/gray-matter-2.1.1.tgz#3042d9adec2a1ded6a7707a9ed2380f8a17a430e"
@@ -5466,7 +5460,7 @@ [email protected]:
   version "2.1.1"
   resolved "https://registry.yarnpkg.com/items/-/items-2.1.1.tgz#8bd16d9c83b19529de5aea321acaada78364a198"
 
-[email protected], iterall@^1.1.0:
+iterall@^1.1.0:
   version "1.1.3"
   resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.1.3.tgz#1cbbff96204056dde6656e2ed2e2226d0e6d72c9"

@efallancy
Copy link
Contributor Author

@bvaughn that's an interesting catch. Yup, I can update the PR to add "eslint-plugin-relay/graphql": "0.10.5" 😄

I'm using yarn 1.2.1

@bvaughn
Copy link
Contributor

bvaughn commented Nov 1, 2017

Just updated to Yarn 1.2.1 myself and was able to yarn build successfully. That's interesting.

May still best to add this resolution for now, to avoid tripping up others? ☹️

In the meanwhile, I also added a note to graphql/graphql-js/pull/989

@efallancy
Copy link
Contributor Author

efallancy commented Nov 2, 2017

@bvaughn interesting yarn is interesting 🤣 Thanks for the updates.

Based on the notes that you added at graphql/graphql-js#989, I think it is still best to add the resolution, since it used to occur before.

@bvaughn
Copy link
Contributor

bvaughn commented Nov 2, 2017

Agreed.

I also spoke with Lee out of band and I don't think that PR is going to land because it apparently doesn't solve the problem so much as it moves it.

@efallancy
Copy link
Contributor Author

Hi @bvaughn, pardon for the delay. I've updated the package.json with the requested change on adding resolution. Please confirm that it's working fine on your end as well 😃

I ran the same command and build succeeded without any error. 🙇

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Fetch and did a clean install+build locally just now and it looks good. Let's move forward with this fix. 👍

@bvaughn bvaughn merged commit 162ce4f into reactjs:master Nov 2, 2017
jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants