Skip to content

Commit e2d0402

Browse files
committed
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.
1 parent 6253b8f commit e2d0402

File tree

2 files changed

+57
-50
lines changed

2 files changed

+57
-50
lines changed

packages/emotion/src/styled.test.tsx

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,28 @@ const SpaceTheme = ({ children }: { children: React.ReactNode }) => {
1313
}
1414

1515
describe('#styled', () => {
16+
it('supports basic tags', () => {
17+
const Dummy = styled.div``
18+
const { container } = render(<Dummy />)
19+
expect(container.firstChild!.nodeName).toBe('DIV')
20+
})
21+
22+
it('passes options through', () => {
23+
// https://emotion.sh/docs/styled#customizing-prop-forwarding
24+
const Dummy = styled('div', {
25+
shouldForwardProp: (prop) => prop !== 'foo',
26+
})``
27+
// @ts-ignore
28+
const { container } = render(<Dummy foo="one" bar="two" />)
29+
expect(container.firstChild).not.toHaveAttribute('foo', 'one')
30+
expect(container.firstChild).toHaveAttribute('bar', 'two')
31+
})
32+
})
33+
34+
describe.each([['div'], ['box']])('#styled.%s', (key) => {
1635
it('transforms rules', () => {
17-
const Dummy = styled.div`
36+
// @ts-ignore
37+
const Dummy = styled[key]`
1838
margin: 2;
1939
padding: 1;
2040
margin-top: 2px;
@@ -34,11 +54,15 @@ describe('#styled', () => {
3454
interface DummyProps {
3555
margin: number
3656
}
37-
const Dummy = styled.div<DummyProps>`
57+
// @ts-ignore
58+
const Dummy = styled[key]<DummyProps>`
3859
color: red;
39-
${(p) => css`
40-
margin: ${p.margin};
41-
`}
60+
${
61+
// @ts-ignore
62+
(p) => css`
63+
margin: ${p.margin};
64+
`
65+
}
4266
`
4367
const { container } = render(
4468
<SpaceTheme>
@@ -61,7 +85,8 @@ describe('#styled', () => {
6185
transform: translateX(100%);
6286
}
6387
`
64-
const Dummy = styled.div`
88+
// @ts-ignore
89+
const Dummy = styled[key]`
6590
${css`
6691
animation: ${animation};
6792
`}
@@ -78,7 +103,8 @@ describe('#styled', () => {
78103
primary: 'pink',
79104
},
80105
}
81-
const Dummy = styled.div`
106+
// @ts-ignore
107+
const Dummy = styled[key]`
82108
color: primary;
83109
`
84110
const { container } = render(
@@ -95,7 +121,8 @@ describe('#styled', () => {
95121
md: 10,
96122
},
97123
}
98-
const Dummy = styled.div`
124+
// @ts-ignore
125+
const Dummy = styled[key]`
99126
margin: -md;
100127
`
101128
const { container } = render(
@@ -107,7 +134,8 @@ describe('#styled', () => {
107134
})
108135

109136
it('works with css as object', () => {
110-
const Dummy = styled.div({
137+
// @ts-ignore
138+
const Dummy = styled[key]({
111139
margin: '2',
112140
})
113141
const { container } = render(
@@ -119,7 +147,8 @@ describe('#styled', () => {
119147
})
120148

121149
it('works with css as object and function prop', () => {
122-
const Dummy = styled.div(() => ({
150+
// @ts-ignore
151+
const Dummy = styled[key](() => ({
123152
margin: '2',
124153
}))
125154
const { container } = render(
@@ -131,7 +160,8 @@ describe('#styled', () => {
131160
})
132161

133162
it('transforms first class interpolations', () => {
134-
const Dummy = styled.div`
163+
// @ts-ignore
164+
const Dummy = styled[key]`
135165
${() => [
136166
'color: red;',
137167
css`
@@ -143,20 +173,13 @@ describe('#styled', () => {
143173
expect(container.firstChild).toHaveStyle('color: red; margin: 4px;')
144174
})
145175

146-
it('passes options through', () => {
147-
// https://emotion.sh/docs/styled#customizing-prop-forwarding
148-
const Dummy = styled('div', {
149-
shouldForwardProp: (prop) => prop !== 'color',
150-
})``
151-
const { container } = render(<Dummy color="lemonchiffon" />)
152-
expect(container.firstChild).not.toHaveAttribute('color', 'lemonchiffon')
153-
})
154-
155176
it('should not pass props that are invalid html attributes', () => {
156177
// https://emotion.sh/docs/styled#customizing-prop-forwarding
157-
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(jest.fn())
178+
const consoleErrorSpy = jest
179+
.spyOn(console, 'error')
180+
.mockImplementation(jest.fn())
158181
const Dummy = styled.box({})
159-
182+
160183
// @ts-ignore
161184
const { container } = render(<Dummy $dark={false} />)
162185

@@ -167,14 +190,6 @@ describe('#styled', () => {
167190
})
168191
})
169192

170-
describe('#styled.xxx', () => {
171-
it('supports basic tags', () => {
172-
const Dummy = styled.div``
173-
const { container } = render(<Dummy />)
174-
expect(container.firstChild!.nodeName).toBe('DIV')
175-
})
176-
})
177-
178193
describe('#styled.xxxBox', () => {
179194
it('supports box tags', () => {
180195
const Dummy = styled.box``

packages/emotion/src/styled.ts

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,23 @@ function flattenArgs(arg: any, props: any): any {
1414
}
1515

1616
function getCreateStyle(baseCreateStyle: any, ...generators: StyleGenerator[]) {
17-
return (strings: any, ...args: any) =>
18-
baseCreateStyle((props: any) => {
19-
let flattenedArgs = flattenArgs(args, props)
20-
21-
// Emotion's css function can receive: template literal (array of
22-
// strings followed by interpolations), style object, or array of style
23-
// objects. Additional generators supplied to getCreateStyle need to be
24-
// interpolated differently depending on which form is called.
25-
if (generators.length) {
26-
if (Array.isArray(strings) && typeof strings[0] === 'string') {
17+
const createStyle = generators.length
18+
? (strings: any, ...args: any) => {
19+
if (Array.isArray(strings)) {
2720
// The tagged template literal should receive an equal number of
2821
// additional separators.
2922
strings = strings.concat(generators.map(() => '\n'))
30-
flattenedArgs = flattenedArgs.concat(flattenArgs(generators, props))
31-
} else if (Array.isArray(strings)) {
32-
// Resolve generators to objects and append to existing array.
33-
strings = strings.concat(flattenArgs(generators, props))
34-
} else {
35-
// Convert single object to array.
36-
strings = [strings].concat(flattenArgs(generators, props))
3723
}
24+
args = args.concat(generators)
25+
return baseCreateStyle((props: any) =>
26+
css(strings, ...flattenArgs(args, props))(props),
27+
)
3828
}
39-
40-
return css(strings, ...flattenedArgs)(props)
41-
})
29+
: (strings: any, ...args: any) =>
30+
baseCreateStyle((props: any) =>
31+
css(strings, ...flattenArgs(args, props))(props),
32+
)
33+
return createStyle
4234
}
4335

4436
type BoxStyledTags = {

0 commit comments

Comments
 (0)