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(graphiql): add react 17, 18 in peerDependencies #1934

Merged
merged 2 commits into from Oct 29, 2021

Conversation

tonyfromundefined
Copy link
Contributor

@tonyfromundefined tonyfromundefined commented Aug 4, 2021

  • For fix this warning, add react 17, 18 in peerDependencies of graphiql

스크린샷 2021-08-04 오후 2 37 38

@changeset-bot
Copy link

changeset-bot bot commented Aug 4, 2021

🦋 Changeset detected

Latest commit: 127e4c2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
graphiql Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #1934 (127e4c2) into main (2d91916) will increase coverage by 0.13%.
The diff coverage is 65.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1934      +/-   ##
==========================================
+ Coverage   65.70%   65.84%   +0.13%     
==========================================
  Files          85       86       +1     
  Lines        5106     5135      +29     
  Branches     1631     1638       +7     
==========================================
+ Hits         3355     3381      +26     
- Misses       1747     1750       +3     
  Partials        4        4              
Impacted Files Coverage Δ
...ackages/graphiql-toolkit/src/create-fetcher/lib.ts 50.90% <20.00%> (-8.67%) ⬇️
packages/graphiql/src/utility/HistoryStore.ts 62.26% <62.26%> (ø)
packages/graphiql/src/components/QueryHistory.tsx 73.91% <76.47%> (+6.69%) ⬆️
...iql/src/components/DocExplorer/MarkdownContent.tsx 100.00% <100.00%> (ø)
packages/graphiql/src/components/GraphiQL.tsx 58.34% <100.00%> (+0.83%) ⬆️
...hql-language-service-server/src/findGraphQLTags.ts 64.89% <100.00%> (+4.25%) ⬆️
packages/graphiql/src/utility/QueryStore.ts 42.85% <0.00%> (+2.04%) ⬆️
packages/graphiql/src/components/ToolbarButton.tsx 92.85% <0.00%> (+21.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b3e8c3...127e4c2. Read the comment docs.

@acao
Copy link
Member

acao commented Oct 5, 2021

I haven’t worked with react 17 or 18 yet, considering we are still using methods which are deprecated in 16, do we know if these versions are compatible with 1.x for certain, before the hooks rewrite?

@acao acao added the graphiql label Oct 13, 2021
@vihka-spihka
Copy link

Hello @acao ,

Before I upgraded my npm to the higher version (that doesn't throw errors regarding peer dependencies, but warnings), I was able to work with the following set without any issues:

  • graphiql@1.4.2,
  • graphql@15.6.1,
  • react@17.0.2
  • react-dom@17.0.2.

Of cause, right after the upgrade of npm, first I got a peer dependency error regarding the graphql, which was easily resolved by downgrading to graphql@15.5.0.
Right after that I got an error regarding the react@17.0.2, and, unfortunately, I can't downgrade the react on my project, so, this issue stays unresolved.

As far as I can see from the React website, they left all the deprecated methods with UNSAFE_ prefix in version 17. I've checked graphiql repo, and found that you already use the prefix.

As for React 18, it hasn't been released yet, so, I can't tell you more about it.

Hope, my comment will help you in your decision regarding the PR.

@acao
Copy link
Member

acao commented Oct 29, 2021

thanks for your validation! I think we can consider this good to go then! thanks everyone :)

@acao acao merged commit d3a8828 into graphql:main Oct 29, 2021
@github-actions github-actions bot mentioned this pull request Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants