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
Updated peerDependency to account for styled-components v4. #184
Updated peerDependency to account for styled-components v4. #184
Conversation
something weird happened on the travis side. I don't think this is related to my changes. |
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.
Thanks for working on this, @alansouzati.
I added a couple of comments.
@@ -1 +1,2 @@ | |||
node_modules | |||
package-lock.json |
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 shouldn't be needed, as there's no package-lock.json
in this repo.
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.
the problem is when I run npm install
it gets added.
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 think your recommendation is always to use yarn install
since you are providing the yarn.lock file. sorry I missed that.
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.
Yes, that would be the best given we have a yarn.lock
.
@@ -58,7 +56,7 @@ | |||
"css": "^2.2.4" | |||
}, | |||
"peerDependencies": { | |||
"styled-components": "^2.0.0 || ^3.0.2" | |||
"styled-components": "^2.0.0 || ^3.0.2 || >= 4.x" |
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.
Using babel-preset-env
is OK, but this library is not ready yet for supporting v4.
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 see. I'm not using enzyme, it is working with react-test-renderer
. But I understand why you want to wait on adding that as a peerDependency.
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.
Correct.
I was thinking about deprecate Enzyme, but I believe a lot of people find it useful.
@MicheleBertoli what is the status on this? Would really appreciate an update :) |
Ditto what @spawnia asked. When will this support styled-components@v4? What needs to be done before this PR can be merged? @MicheleBertoli Can you add a list of tasks to be completed before this PR is mergeable? FYI - I'm using this with react-testing-library instead of Enzyme or |
I second @beausmith and @spawnia's comments. I don't think this library will be useful without supporting ^4.0+ |
I think releasing a new major version that doesn't officially support enzyme while we get over those issues would be a good idea. Then we can do a quick release with the fixes. This library works fine with react-testing-library and styled-components v4 |
@MicheleBertoli what is the status on this? I'm working with mount of enzyme for write my tests with success. With this PR, I can rewrite using shallow. |
No longer needed after general repo maintenance earlier today, thanks for your PR though! Just waiting to get added as an owner on npm so I can actually publish an update. |
Also updated babel config to use
env
preset to avoid warning in the console when using the deprecatedes2015
preset.