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

bump react & react-dom to 16.8 #7883

Conversation

nisarhassan12
Copy link
Contributor

@nisarhassan12 nisarhassan12 commented May 25, 2020

What it does

Fixes #7879

bumps react & react-dom to 16.8 as @akosyakov said in #7879 (comment) :

React hooks were only introduced in https://reactjs.org/blog/2019/02/06/react-v16.8.0.html but we make use them with early version in package.json. We should bump it to make sure that downstreams update their yarn.lock files.

How to test

Review checklist

Reminder for reviewers

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@nisarhassan12 thank you for your contribution!

Can you please sign-off your commit (reason for the failed CI check) and also fill out the pull-request description template?

@vince-fugnitto vince-fugnitto added dependencies pull requests that update a dependency file react issues related to the react language labels May 25, 2020
@nisarhassan12 nisarhassan12 force-pushed the nisarhassan12/set-minimal-react-7879 branch from e20b5ee to 9691bda Compare May 25, 2020 15:01
@nisarhassan12
Copy link
Contributor Author

Thanks! @vince-fugnitto I have signed of the commit and filled the pull request description template.

@vince-fugnitto
Copy link
Member

Thanks! @vince-fugnitto I have signed of the commit and filled the pull request description template.

I meant 'what it does' and 'how to test', we use this fields during reviews but also for traceability if we ever need to revisit the pull-request in the future.

@nisarhassan12
Copy link
Contributor Author

@vince-fugnitto Opps Sorry.

@nisarhassan12
Copy link
Contributor Author

@vince-fugnitto Done. Sorry for my bad communication skills.

@kittaakos
Copy link
Contributor

Why not ^16.8? Is there a reason for pinning it?

@nisarhassan12 nisarhassan12 force-pushed the nisarhassan12/set-minimal-react-7879 branch 4 times, most recently from 44000c6 to 53a2193 Compare May 26, 2020 10:08
@akosyakov
Copy link
Member

@nisarhassan12 please make sure to build the project and fix compilation errors.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@nisarhassan12 I see the following error:

packages/navigator/src/browser/navigator-widget.tsx:236:38 - error TS2345: Argument of type 'import("/Users/vincentfugnitto/workspace/theia/node_modules/@types/react/index").MouseEvent<HTMLElement, MouseEvent>' is not assignable to parameter of type 'React.MouseEvent<HTMLElement, MouseEvent>'.
  Type 'MouseEvent<HTMLElement, MouseEvent>' is missing the following properties from type 'MouseEvent<HTMLElement, MouseEvent>': detail, view

236         super.handleClickEvent(node, event);

@nisarhassan12 nisarhassan12 force-pushed the nisarhassan12/set-minimal-react-7879 branch 2 times, most recently from fca1097 to 649ac94 Compare May 27, 2020 02:34
@nisarhassan12
Copy link
Contributor Author

Thanks! the project can be successfully built I have fixed the compilation errors.

yarn.lock Outdated Show resolved Hide resolved
@nisarhassan12 nisarhassan12 force-pushed the nisarhassan12/set-minimal-react-7879 branch 3 times, most recently from 2a15b17 to 5915b90 Compare May 27, 2020 11:56
Signed-off-by: Nisar Hassan Naqvi <syednisarhassan12@gmail.com>
@nisarhassan12 nisarhassan12 force-pushed the nisarhassan12/set-minimal-react-7879 branch from 5915b90 to 6dc2082 Compare May 27, 2020 17:20
@akosyakov
Copy link
Member

@vince-fugnitto Could you test it please? Code looks good.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@akosyakov sorry I didn't see the ping, I verified and did not notice any regressions 👍

@akosyakov akosyakov merged commit 310ff52 into eclipse-theia:master Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file react issues related to the react language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set minimal react version as 16.8
4 participants