Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

fix(react): add react-compat to deal with the warnings #32

Merged
merged 1 commit into from
Apr 9, 2017
Merged

fix(react): add react-compat to deal with the warnings #32

merged 1 commit into from
Apr 9, 2017

Conversation

kentcdodds
Copy link
Contributor

React@15.5.0 introduced deprecation warnings for a few things and they're really annoying. So this will help prevent those warnings from logging for people using this package.

@codecov-io
Copy link

codecov-io commented Apr 7, 2017 •
edited
Loading

Codecov Report

Merging #32 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #32   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          62     61    -1     
  Branches        9      9           
=====================================
- Hits           62     61    -1
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️

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 7623088...de50b7c. Read the comment docs.

kentcdodds
kentcdodds commented Apr 7, 2017
View reviewed changes
README.md Outdated
@@ -132,6 +135,7 @@ If you want to use the global:
```html
<!-- Load dependencies -->
<script src="https://unpkg.com/react/dist/react.js"></script>
<script src="https://packd.now.sh/prop-types?name=PropTypes"></script>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is assuming the service works (which it's not right now) and it has no SLA. Not a huge deal because this is really only for demos and toys anyway...

kentcdodds
kentcdodds commented Apr 8, 2017
View reviewed changes
package.json Outdated
"react-dom": "^15.4.2",
"prop-types": "^15.5.1",
"react": "^15.5.1",
"react-addons-test-utils": "^15.5.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is still required until this gets merged: enzymejs/enzyme#876

kentcdodds
kentcdodds commented Apr 8, 2017
View reviewed changes
rollup.config.js
globals: {
react: 'React',
glamor: 'Glamor',
'prop-types': 'PropTypes',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm making this up. prop-types is not currently available via UMD.

Copy link

Choose a reason for hiding this comment

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

It is. There are UMD files inside of it. cc @acdlite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are now :) Thanks @acdlite :)

fix(react): add react-compat to deal with the warnings
de50b7c 
React@15.5.0 introduced deprecation warnings for a few things and
they're really annoying. So this will help prevent those warnings from
logging for people using this package.
@paulmolluzzo
Copy link
Collaborator

Will review this tonight @kentcdodds.

@paulmolluzzo
Copy link
Collaborator

I'm getting these errors after running npm start validate:

[test]   console.error node_modules/fbjs/lib/warning.js:36
[test]     Warning: ReactTestUtils has been moved to react-dom/test-utils. Update references to remove this warning.
[test] 
[test]   console.error node_modules/fbjs/lib/warning.js:36
[test]     Warning: ReactTestUtils has been moved to react-dom/test-utils. Update references to remove this warning.

screen shot 2017-04-09 at 3 01 26 pm

paulmolluzzo
paulmolluzzo reviewed Apr 9, 2017
View reviewed changes
Copy link
Collaborator

@paulmolluzzo paulmolluzzo left a comment

Choose a reason for hiding this comment

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

Commented with an error I'm seeing:

[test]   console.error node_modules/fbjs/lib/warning.js:36
[test]     Warning: ReactTestUtils has been moved to react-dom/test-utils. Update references to remove this warning.
[test] 
[test]   console.error node_modules/fbjs/lib/warning.js:36
[test]     Warning: ReactTestUtils has been moved to react-dom/test-utils. Update references to remove this warning.

Also, this branch is behind master by 1 commit.

paulmolluzzo
paulmolluzzo suggested changes Apr 9, 2017
View reviewed changes
Copy link
Collaborator

@paulmolluzzo paulmolluzzo left a comment

Choose a reason for hiding this comment

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

Another thing: I'm concerned about merging this with the dependency CI throwing an error. All PRs will have an error until your other PR is merged. I'll defer to your decision here.

@kentcdodds
Copy link
Contributor Author

That error is coming from enzyme. Will be fixed soon: enzymejs/enzyme#876

Don't worry about dependency CI. I actually fixed that a bit ago. Just takes time for dependency CI to pick up on the change.

@kentcdodds kentcdodds merged commit 14970c1 into master Apr 9, 2017
@kentcdodds kentcdodds deleted the pr/react-compat branch April 9, 2017 21:58
@rpominov rpominov mentioned this pull request May 5, 2017
Closed
@atticoos atticoos mentioned this pull request Jul 28, 2017
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gaearon gaearon gaearon left review comments

@paulmolluzzo paulmolluzzo paulmolluzzo requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kentcdodds @codecov-io @paulmolluzzo @gaearon