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

Add warning for non styled components #2070

Merged
merged 3 commits into from Oct 8, 2018

Conversation

Projects
None yet
3 participants
@egdbear
Member

egdbear commented Oct 7, 2018

#2053

Hello, i think i made a fix for showing right warning message.
Please feel free to suggest better error message :)

@probablyup

A couple things:

  1. we only want this to happen in development because it’s a hot path, so needs a process.env.NODE_ENV !== “production” wrapper

  2. this should be a warning not an error imo. An error implies a fatal problem, where this will just produce invalid css.

@probablyup

This comment has been minimized.

Contributor

probablyup commented Oct 7, 2018

Also need a test for it

@egdbear egdbear force-pushed the egdbear:add-warning-non-styled-component branch from b576cf6 to b613570 Oct 7, 2018

@egdbear

This comment has been minimized.

Member

egdbear commented Oct 7, 2018

@probablyup Hopefully now makes more sense :)

/* Warn if not refring styled component */
if (process.env.NODE_ENV !== 'production') {
if (ReactIs.isElement(chunk)) {
return console.warn(

This comment has been minimized.

@probablyup

probablyup Oct 7, 2018

Contributor
  1. I liked what you were doing before in calling the function and then checking the return to see if it was JSX. That felt more safe to me.

  2. Let's not return, that'll cause other issues. Just the warning is fine. And let's update the text slightly:

${getComponentName(chunk)} is not a styled component and cannot be referred to via component selector. See https://www.styled-components.com/docs/advanced#referring-to-other-components for more details.

getComponentName comes from src/utils/getComponentName.js

@egdbear egdbear force-pushed the egdbear:add-warning-non-styled-component branch from 72f96f8 to 8d552fd Oct 8, 2018

@egdbear egdbear force-pushed the egdbear:add-warning-non-styled-component branch from 0ee001c to 77133ae Oct 8, 2018

@probablyup

Looking great! Just the one nit and then I think it looks good to merge.

@@ -99,4 +102,22 @@ describe('flatten', () => {
expect(flatten(['foo', func], { bool: true })).toEqual(['foo', 'static', 'bar']);
expect(flatten(['foo', func], { bool: false })).toEqual(['foo', 'static', 'baz']);
});
it('warns to refer styled-components', () => {

This comment has been minimized.

@probablyup

probablyup Oct 8, 2018

Contributor

nit: can you change this test name to "warns if trying to interpolate a normal React component"

egdbear

@egdbear egdbear force-pushed the egdbear:add-warning-non-styled-component branch from df0fb41 to 843bb6a Oct 8, 2018

@probablyup

Awesome, thank you!

@egdbear

This comment has been minimized.

Member

egdbear commented Oct 8, 2018

@probablyup you are welcome, thanks for comments as well!

@probablyup

This comment has been minimized.

Contributor

probablyup commented Oct 8, 2018

Last thing, want to add a changelog message? :)

egdbear
@probablyup

This comment has been minimized.

Contributor

probablyup commented Oct 8, 2018

Thanks again!

@probablyup probablyup merged commit bebd244 into styled-components:develop Oct 8, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@mxstbr

This comment has been minimized.

Member

mxstbr commented Oct 8, 2018

Thank you so much for helping us improve styled-components! Based on our Community Guidelines every person that has a PR of any kind merged is offered an invitation to the styled-components organization—that is you right now!

Should you accept, you'll get write access to the main repository. (and a fancy styled-components logo on your GitHub profile!) You'll be able to label and close issues, merge pull requests, create new branches, etc. We want you to help us out with those things, so don't be scared to do them! Make sure to read our contribution and community guidelines, which explains all of this in more detail. Of course, if you have any questions just let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment