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

Updated regex to match attribute change of v4 #181

Merged
merged 1 commit into from
Sep 8, 2018
Merged

Updated regex to match attribute change of v4 #181

merged 1 commit into from
Sep 8, 2018

Conversation

fbring
Copy link
Contributor

@fbring fbring commented Sep 6, 2018

As already mentioned in #169 there is an attribute update in v4 of styled-components.

Copy link
Member

@MicheleBertoli MicheleBertoli left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks @fbring.
Well done with the tests, and I'm glad you made a backwards-compatible change.

Unfortunately, I tried upgrading to v4 locally and I noticed there are various broken tests, so I need to do a bit more work - but this is covered for now.

I left one comment, and then we can merge.

@@ -18,7 +18,7 @@ if (isStyledComponent) {
StyleSheet = secretInternals.StyleSheet
}
} else {
StyleSheet = require('styled-components/lib/models/StyleSheet').default
StyleSheet = require('styled-components/lib/models/StyleSheet').default // eslint-disable-line
Copy link
Member

Choose a reason for hiding this comment

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

This gives you an error only when you have v4 installed, right?
I couldn't repro with v3.

If that's the case, this is safe to remove, as this PR is not adding v4 yet.

@MicheleBertoli
Copy link
Member

Thank you very much, @fbring.
I'm merging this, and I'll remove the comment as well.
Congrats on your first PR 🚀

@MicheleBertoli MicheleBertoli merged commit 032d085 into styled-components:master Sep 8, 2018
@fbring
Copy link
Contributor Author

fbring commented Sep 8, 2018

Thank you, @MicheleBertoli .

Sorry, yesterday I had no time to look over it again.

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.

2 participants