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

Upgrade dependencies (React, Typescript, Eslint, get rid of Tslint), mark componentWillReceiveProps as Unsafe #95

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

manvydasu
Copy link

Next step would be to replace deprecated UNSAFE_componentWillReceiveProps with something else (#85).

…d in tsconfig.json, fix invalid props being passed to DOM
Copy link

@mshwery mshwery left a comment

Choose a reason for hiding this comment

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

lgtm minus a couple comments!

package.json Outdated
"react": "^17.0.1",
"react-addons-test-utils": "^15.6.2",
"react-dom": "^17.0.1",
"react-test-renderer": "^17.0.1",
Copy link

Choose a reason for hiding this comment

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

unused, can remove

package.json Outdated
"react-test-renderer": "^15.4.2",
"prettier": "^2.2.1",
"react": "^17.0.1",
"react-addons-test-utils": "^15.6.2",
Copy link

Choose a reason for hiding this comment

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

unused, can remove?

package.json Outdated
@@ -23,40 +23,44 @@
"prop-types": "^15.5.7"
},
"peerDependencies": {
"react": "15.x || 16.x"
"react": "15.x || 16.x || 17.x"
Copy link

Choose a reason for hiding this comment

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

15.x won't work with UNSAFE_ methods.

Copy link

Choose a reason for hiding this comment

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

I think min needs to be 16.3 now.

Copy link
Author

Choose a reason for hiding this comment

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

You're right, I forgot that UNSAFEs were added only with 16.3.. Thanks for review, hopefully @clauderic can look into it, since a lot of people have to spend time forking the repo.

Copy link

@Domush Domush left a comment

Choose a reason for hiding this comment

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

A lot of unnecessary Dev dependencies, including prettier and shopify

.eslintrc file is user-specific and doesn't belong in this pull request.

@manvydasu
Copy link
Author

A lot of unnecessary Dev dependencies, including prettier and shopify

.eslintrc file is user-specific and doesn't belong in this pull request.

What do you mean by a 'lot of unnecessary Dev dependencies'? This project is already using shopify's eslint configuration. I just upgraded it to newer version.

and .eslintrc file is not user specific. That's rules for the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants