-
Notifications
You must be signed in to change notification settings - Fork 202
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
remove react-dom dependency #39
Conversation
They are deprecating findDOMNode() in this release. It still exists in React for now, but will eventually only be available in react-dom (as far as I know- so if we are going to use it- then it's better to use the react-dom package IMO). Search for The better fix IMO is removing react-dom like you did here, but stop using Changing: To: Can you make that change, then I'll pull this in and test it. If it works, I'll publish a new version today... |
@@ -5,7 +5,7 @@ | |||
"main": "./lib/index.js", | |||
"scripts": { | |||
"test": "echo \"Error: no test specified\" && exit 1", | |||
"build": "node ./node_modules/gulp/bin/gulp.js build" | |||
"build": "gulp build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure which version I prefer. Not everyone is going to have gulp installed globally, so I kind of like the node
version, since gulp
will be installed to node_modules after running npm install
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't mean gulp has to be installed globally.
NPM will use the local version if installed (as v2 i believe)
cool - didn't know findDOM was being removed. Have updated the PR as requested. Worked fine for me after a rebuild |
], | ||
"engines": { | ||
"npm": "~2.13.0", | ||
"node": "~0.12.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using node 4.0.0 right now, so we should probably change this to >
or use a range or something..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh - straight into 4.0. brave! I've not been able to update yet, soon i hope!
If it works with node4 that a >
makes sense.
remove react-dom dependency
react-dom
is not needed