feat: deprecate support for 15, support react 16 features #1107
Conversation
acao
force-pushed
the
feat/deprecate-react-15
branch
from
27ced8e to
a2d38e3
Compare
- use UNSAFE_ for one componentWillUpdateProps - refact for another componentWillUpdateProps - deprecate peer resolution for react 15 - dev dependencies bumped to react 16 - react-test-renderer and enzyme bumped to react 16 Created-By: @ryan-m-walker, @acao BREAKING CHANGE: Deprecate support for React 15. Please use React 16.8 or greater for hooks support.
acao
force-pushed
the
feat/deprecate-react-15
branch
from
a2d38e3 to
92a4093
Compare
Codecov Report
@@ Coverage Diff @@
## master #1107 +/- ##
==========================================
+ Coverage 44% 45.69% +1.69%
==========================================
Files 64 64
Lines 3000 3040 +40
Branches 650 660 +10
==========================================
+ Hits 1320 1389 +69
+ Misses 1409 1399 -10
+ Partials 271 252 -19
Continue to review full report at Codecov.
|
| @@ -114,6 +89,22 @@ export class QueryHistory extends React.Component { | |||
| ); | |||
| } | |||
|
|
|||
| updateHistory = (query, variables, operationName) => { | |||
There was a problem hiding this comment.
I noticed in other areas that public methods were marked with a Public API comment which I didn't add here. Could you add it in for me?
e.g.:
There was a problem hiding this comment.
sure yes! we are about to deprecate this “plugin api” possibly
There was a problem hiding this comment.
yeah I figured this would go away once we moved to more React 16 patterns anyways but good to make it clear and consistent until then
There was a problem hiding this comment.
yep! this reminded me i need to add a deprecation notice
|
@ryan-m-walker this increases the test coverage for GraphiQL itself by almost 6%, awesome! |
There was a problem hiding this comment.
This seems to be removing support for React 15 rather than deprecating it. If that's the intent, please can you rewrite the PR title and description to make this clear. If it's not then I think we're going to need to be very careful about some of these changes.
| 'GraphiQL 0.18.0 and after is not compatible with React 15 or below.', | ||
| 'If you are using a CDN source (jsdelivr, unpkg, etc), follow this example:', | ||
| 'https://github.com/graphql/graphiql/blob/master/examples/graphiql-cdn/index.html#L49', | ||
| ].join('\n'), |
There was a problem hiding this comment.
This error is misleading; e.g. if they were to use React 17 or 18 in the future.
There was a problem hiding this comment.
we will likely remove this before react 17. i can add a todo for that
| // eslint-disable-next-line no-console | ||
| const logger = console.log; | ||
|
|
||
| if (!React.version || !React.version.indexOf('16') < 0) { |
There was a problem hiding this comment.
Is React.version a public API? I couldn't find it in the React docs (didn't look for long though)
There was a problem hiding this comment.
from very early on its been an available APi!
acao
changed the title
|
@benjie after this PR, GraphiQL will not work with React 15, and we will soon use hooks which means 16.8 will be the minimum |
UNSAFE_for onecomponentWillUpdatePropscomponentWillUpdatePropsvia @ryan-m-walker's PRreact-test-rendererandenzymebumped to react 16BREAKING CHANGE: Deprecate support for React 15. Please use React 16.8 or greater for hooks support.
thanks @ryan-m-walker for doing most of this!