Move React from dependencies to peerDependencies - #93
Conversation
| // expect(validAttr('y2')).toEqual(true) | ||
| // expect(validAttr('yChannelSelector')).toEqual(true) | ||
| // expect(validAttr('z')).toEqual(true) | ||
| // expect(validAttr('zoomAndPan')).toEqual(true) |
There was a problem hiding this comment.
React 0.14 does support some of these but it was really fiddly working out which ones so I took the nuclear option for now.
There was a problem hiding this comment.
Maybe these can be made conditional on React.version?
There was a problem hiding this comment.
I was just thinking about this again and wondered if we need to do anything? If we leave all the tests in and someone clones the repo and runs them then they'll pass because at the moment they get react@15.3.2 from the devDependencies. It's only if they go back to an old React 0.14 version that the tests will fail.
@mxstbr, let me know what you'd prefer to have. I can put these failing tests in a conditional like @wmertens suggested (if (React.version.indexOf('0.14') !== 0) {…} maybe?) or just leave them as they were and they'll still pass on current versions of React 😄
|
I've been wondering if this works as expected, even in the UMD build. Can you test that? |
|
It works as expected for me. React is in the |
|
I think it's a good idea to move the react dependency to |
They are testing that we allow all valid attributes through to the DOM node. That these tests break on react 0.14 means that people will have a horrible experience using styled-components with it, because we won't allow attributes to be attached to the DOM node underneath. Can we maybe inline the react 15.0 list? We should do that anyway, and it'd mean nothing breaks on 0.14! |
|
OK, I've inlined the list from React 15. I think it's the best thing to do, especially after this comment. It should also reduce file size as other things were being pulled in from the imported files that weren't being used. Original tests pass with both React 0.14 and 15. |
|
Awesome, thanks! |
|
Thank you! |
At the moment React is in
dependencieswhich means15.3.2is being forced on everyone who usesstyled-components. They'll end up having two versions of React in theirnode_modulesif they aren't on the same version. I believe this can cause problems with Webpack (this may be fixed now).This change moves React into
peerDependencieslike most other libraries that need React do. I also allowed React 0.14.x to be used and apart from some failing tests invalidAttr(I've commented them out so you can see which ones) everything seemed fine. I believe the failing tests are just because React 15 has a lot better support for SVG and a few newer HTML attributes.React has also been added to
devDependenciesto be more convenient during development and to avoid peer dependency warnings.So the main benefit of all this is more people can use
styled-componentseven if they're on an older version of React.