fix(emotion): broken getCreateStyle

I misunderstood how xstyled's css function interacts with emotion's css function, so I added this complicated code in c400c70. More recently, while working on #250, I discovered that it's also broken, because it converts a function argument into an array. This simpler code does the one thing it needs to do: add string separators for the tagged template literal for the number of generators. It also produces a slightly more efficient result, because it does more work up front (testing `generators.length` and concatting arrays) before the resulting function, rather than every time the function runs.
styled-components · May 4, 2021 · e2d0402 · e2d0402
1 parent 6253b8f
commit e2d0402
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 50 deletions.
diff --git a/packages/emotion/src/styled.test.tsx b/packages/emotion/src/styled.test.tsx
@@ -13,8 +13,28 @@ const SpaceTheme = ({ children }: { children: React.ReactNode }) => {
 }
 
 describe('#styled', () => {
+  it('supports basic tags', () => {
+    const Dummy = styled.div``
+    const { container } = render(<Dummy />)
+    expect(container.firstChild!.nodeName).toBe('DIV')
+  })
+
+  it('passes options through', () => {
+    // https://emotion.sh/docs/styled#customizing-prop-forwarding
+    const Dummy = styled('div', {
+      shouldForwardProp: (prop) => prop !== 'foo',
+    })``
+    // @ts-ignore
+    const { container } = render(<Dummy foo="one" bar="two" />)
+    expect(container.firstChild).not.toHaveAttribute('foo', 'one')
+    expect(container.firstChild).toHaveAttribute('bar', 'two')
+  })
+})
+
+describe.each([['div'], ['box']])('#styled.%s', (key) => {
   it('transforms rules', () => {
-    const Dummy = styled.div`
+    // @ts-ignore
+    const Dummy = styled[key]`
       margin: 2;
       padding: 1;
       margin-top: 2px;
@@ -34,11 +54,15 @@ describe('#styled', () => {
     interface DummyProps {
       margin: number
     }
-    const Dummy = styled.div<DummyProps>`
+    // @ts-ignore
+    const Dummy = styled[key]<DummyProps>`
       color: red;
-      ${(p) => css`
-        margin: ${p.margin};
-      `}
+      ${
+        // @ts-ignore
+        (p) => css`
+          margin: ${p.margin};
+        `
+      }
     `
     const { container } = render(
       <SpaceTheme>
@@ -61,7 +85,8 @@ describe('#styled', () => {
         transform: translateX(100%);
       }
     `
-    const Dummy = styled.div`
+    // @ts-ignore
+    const Dummy = styled[key]`
       ${css`
         animation: ${animation};
       `}
@@ -78,7 +103,8 @@ describe('#styled', () => {
         primary: 'pink',
       },
     }
-    const Dummy = styled.div`
+    // @ts-ignore
+    const Dummy = styled[key]`
       color: primary;
     `
     const { container } = render(
@@ -95,7 +121,8 @@ describe('#styled', () => {
         md: 10,
       },
     }
-    const Dummy = styled.div`
+    // @ts-ignore
+    const Dummy = styled[key]`
       margin: -md;
     `
     const { container } = render(
@@ -107,7 +134,8 @@ describe('#styled', () => {
   })
 
   it('works with css as object', () => {
-    const Dummy = styled.div({
+    // @ts-ignore
+    const Dummy = styled[key]({
       margin: '2',
     })
     const { container } = render(
@@ -119,7 +147,8 @@ describe('#styled', () => {
   })
 
   it('works with css as object and function prop', () => {
-    const Dummy = styled.div(() => ({
+    // @ts-ignore
+    const Dummy = styled[key](() => ({
       margin: '2',
     }))
     const { container } = render(
@@ -131,7 +160,8 @@ describe('#styled', () => {
   })
 
   it('transforms first class interpolations', () => {
-    const Dummy = styled.div`
+    // @ts-ignore
+    const Dummy = styled[key]`
       ${() => [
         'color: red;',
         css`
@@ -143,20 +173,13 @@ describe('#styled', () => {
     expect(container.firstChild).toHaveStyle('color: red; margin: 4px;')
   })
 
-  it('passes options through', () => {
-    // https://emotion.sh/docs/styled#customizing-prop-forwarding
-    const Dummy = styled('div', {
-      shouldForwardProp: (prop) => prop !== 'color',
-    })``
-    const { container } = render(<Dummy color="lemonchiffon" />)
-    expect(container.firstChild).not.toHaveAttribute('color', 'lemonchiffon')
-  })
-
   it('should not pass props that are invalid html attributes', () => {
     // https://emotion.sh/docs/styled#customizing-prop-forwarding
-    const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(jest.fn())
+    const consoleErrorSpy = jest
+      .spyOn(console, 'error')
+      .mockImplementation(jest.fn())
     const Dummy = styled.box({})
-    
+
     // @ts-ignore
     const { container } = render(<Dummy $dark={false} />)
 
@@ -167,14 +190,6 @@ describe('#styled', () => {
   })
 })
 
-describe('#styled.xxx', () => {
-  it('supports basic tags', () => {
-    const Dummy = styled.div``
-    const { container } = render(<Dummy />)
-    expect(container.firstChild!.nodeName).toBe('DIV')
-  })
-})
-
 describe('#styled.xxxBox', () => {
   it('supports box tags', () => {
     const Dummy = styled.box``

diff --git a/packages/emotion/src/styled.ts b/packages/emotion/src/styled.ts
@@ -14,31 +14,23 @@ function flattenArgs(arg: any, props: any): any {
 }
 
 function getCreateStyle(baseCreateStyle: any, ...generators: StyleGenerator[]) {
-  return (strings: any, ...args: any) =>
-    baseCreateStyle((props: any) => {
-      let flattenedArgs = flattenArgs(args, props)
-
-      // Emotion's css function can receive: template literal (array of
-      // strings followed by interpolations), style object, or array of style
-      // objects. Additional generators supplied to getCreateStyle need to be
-      // interpolated differently depending on which form is called.
-      if (generators.length) {
-        if (Array.isArray(strings) && typeof strings[0] === 'string') {
+  const createStyle = generators.length
+    ? (strings: any, ...args: any) => {
+        if (Array.isArray(strings)) {
           // The tagged template literal should receive an equal number of
           // additional separators.
           strings = strings.concat(generators.map(() => '\n'))
-          flattenedArgs = flattenedArgs.concat(flattenArgs(generators, props))
-        } else if (Array.isArray(strings)) {
-          // Resolve generators to objects and append to existing array.
-          strings = strings.concat(flattenArgs(generators, props))
-        } else {
-          // Convert single object to array.
-          strings = [strings].concat(flattenArgs(generators, props))
         }
+        args = args.concat(generators)
+        return baseCreateStyle((props: any) =>
+          css(strings, ...flattenArgs(args, props))(props),
+        )
       }
-
-      return css(strings, ...flattenedArgs)(props)
-    })
+    : (strings: any, ...args: any) =>
+        baseCreateStyle((props: any) =>
+          css(strings, ...flattenArgs(args, props))(props),
+        )
+  return createStyle
 }
 
 type BoxStyledTags = {