feat: don't force plain themes

[phyllisstein]

is-plain-object is still required as an additional dependency, in order to guarantee that objects within rulesets are toStringed.

Fixes #1776.

[quantizor]

Looks good. Could you add a test that simulates what webpack does to the best of your ability so we know it'll work?

[quantizor]

@phyllisstein try merging from master - I turned off danger for the moment. Please do add a CHANGELOG entry though 😃

[kitten]

I think we should use this opportunity to get rid of such a small dependency that will likely never change. I think it’d make sense to check for:

• not equal null
• not an array
• is type of object

Without taking on a small dependency. I’m not even sure if we still need this constraint even?

[phyllisstein]

Since this test just passed, I take it SC can pretty well handle non-plain objects:

  it('should [not] allow other complex objects to be passed as themes', () => {
    class Theme {
      constructor(borderRadius) {
        this.borderRadius = borderRadius
      }
    }

    const theme = new Theme('2px')

    const Comp1 = styled.div`
      border-radius: ${ ({ theme }) => theme.borderRadius };
    `

    const wrapper = mount(
      <ThemeProvider theme={ theme }>
        <Comp1 />
      </ThemeProvider>
    )

    expectCSSMatches(`.sc-a {} .b {border-radius:${theme.borderRadius};}`)
  })

I'd be happy to remove the check---and the dependency---altogether. If you want to keep it, though, isobject isn't the right module.

[quantizor]

If what we have already is enough, let's drop the dep.

[phyllisstein]

Consider it done! With a few extra tests to boot.

[quantizor]

@phyllisstein great! could you please merge from master to fix CI and add a changelog entry?

[phyllisstein]

With pleasure. Should be ready to go once tests pass.

[phyllisstein]

Looks like there were some Flow issues that I'll work on correcting, but also a problem with the bundle size check that I'm less sure how to resolve.

[quantizor]

Heh, Flow is doing its job and preventing you from making the mistakes! Since these tests are intentional, you can put a // $FlowInvalidInputTest above the lines in question to disable Flow for that particular case.

[phyllisstein]

Thanks! With that added and a small bump to the maximum bundle size I think I'm set.

[quantizor]

Hmm why'd we have to bump this? Aren't we using less code now?

[phyllisstein]

The check was failing pretty consistently, but I'm not sure where the extra code came from. I guess the typecheck @kitten suggested ((mergedTheme === null || Array.isArray(mergedTheme) || typeof mergedTheme !== 'object')) is slightly longer than the original isPlainObject(mergedTheme), but I'm not sure if that would've been sufficient to break the test.

[quantizor]

Can this dependency be removed entirely now?

[phyllisstein]

It's also used in utils/flatten.js to determine whether to try flattening the object or to just call toString on it. I think in that case it's still necessary that the object be "plain."

[quantizor]

Thank you!

[mxstbr]

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!

[quantizor]

Released in https://github.com/styled-components/styled-components/releases/tag/v3.3.2!

[phyllisstein]

Neat-o! Thanks for all your help!