Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/serialize/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ function createStringFromObject(
} else {
for (let key in obj) {
let value = obj[key]
if (typeof value === 'function' && mergedProps !== undefined) {
value = value(mergedProps)
}
if (typeof value !== 'object') {
if (registered != null && registered[value] !== undefined) {
string += `${key}{${registered[value]}}`
Expand Down
37 changes: 23 additions & 14 deletions packages/serialize/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,23 @@ import * as CSS from 'csstype'
export { RegisteredCache, SerializedStyles }

export type CSSProperties = CSS.PropertiesFallback<number | string>
export type CSSPropertiesWithMultiValues = {
export type CSSPropertiesWithMultiValues<Props = unknown> = {
[K in keyof CSSProperties]:
| CSSProperties[K]
| ReadonlyArray<Extract<CSSProperties[K], string>>
| ((
props: Props
) => CSSProperties[K] | ReadonlyArray<Extract<CSSProperties[K], string>>)
}

export type CSSPseudos = { [K in CSS.Pseudos]?: CSSObject }
export type CSSPseudos<Props = unknown> = {
[K in CSS.Pseudos]?: CSSObject<Props>
}

export interface ArrayCSSInterpolation
extends ReadonlyArray<CSSInterpolation> {}
export interface ArrayCSSInterpolation<Props = unknown>
extends ReadonlyArray<CSSInterpolation<Props>> {}

export type InterpolationPrimitive =
export type InterpolationPrimitive<Props = unknown> =
| null
| undefined
| boolean
Expand All @@ -27,18 +32,22 @@ export type InterpolationPrimitive =
| ComponentSelector
| Keyframes
| SerializedStyles
| CSSObject
| CSSObject<Props>

export type CSSInterpolation = InterpolationPrimitive | ArrayCSSInterpolation
export type CSSInterpolation<Props = unknown> =
Copy link
Member

Choose a reason for hiding this comment

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

This one actually shouldn't end up including functions in CSSObject. We have 2 flavors of those interpolations:

  • the ones that u can pass to css
  • and the ones that you can pass to styled and similar

The difference is that css is a context-less call. We don't get access to any props there (and we don't defer css resolution) so this should be an error:

css({
  display: () => 'flex'
})

That's why we have both ArrayInterpolation and ArrayCSSInterpolation here. A similar distinction should be introduced for those object styles.

But now... I'm not sure if this would satisfy your original goal.

Copy link
Author

Choose a reason for hiding this comment

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

I see, I will revert this. I think it should be fine since we focus only for styled.

Copy link
Author

@siriwatknp siriwatknp Jan 9, 2024

Choose a reason for hiding this comment

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

Done! see 2fb22d4

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be fine since we focus only for styled.

Cool! ❤️

Done! see 2fb22d4

Thanks. I'm soft-approving this PR now. I think we still need to tweak types a little bit but honestly - I need to give it more thought to determine how to do it in the least disruptive way. So I think, it's best for me to do it soon-ish since I don't have concrete advice right now on how this should be done.

I'll also give @emmatown a couple of days to give their opinion about this. If we don't merge it this week - please ping me later.

| InterpolationPrimitive<Props>
| ArrayCSSInterpolation<Props>

export interface CSSOthersObject {
[propertiesName: string]: CSSInterpolation
export interface CSSOthersObject<Props = unknown> {
[propertiesName: string]:
| CSSInterpolation<Props>
| ((props: Props) => CSSInterpolation<Props>)
}

export interface CSSObject
extends CSSPropertiesWithMultiValues,
CSSPseudos,
CSSOthersObject {}
export interface CSSObject<Props = unknown>
extends CSSPropertiesWithMultiValues<Props>,
CSSPseudos<Props>,
CSSOthersObject<Props> {}

export interface ComponentSelector {
__emotion_styles: any
Expand All @@ -59,7 +68,7 @@ export interface FunctionInterpolation<Props> {
}

export type Interpolation<Props> =
| InterpolationPrimitive
| InterpolationPrimitive<Props>
| ArrayInterpolation<Props>
| FunctionInterpolation<Props>

Expand Down
14 changes: 14 additions & 0 deletions packages/serialize/types/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ serializeStyles(
{}
)
// $ExpectType SerializedStyles
serializeStyles<{ vars: { background: string; foreground: string } }>([
{
display: () => ['-webkit-flex', 'flex'],
backgroundColor: ({ vars }) => vars.background,
color: ({ vars }) => vars.foreground,
lineHeight: ({ vars }) => 1.2,
'--css-var': ({ vars }) => vars.foreground,
'&:hover': {
backgroundColor: ({ vars }) => vars.foreground,
color: ({ vars }) => vars.background
}
}
])
// $ExpectType SerializedStyles
serializeStyles([testTemplateStringsArray, 5, '4px'], {}, {})

// $ExpectError
Expand Down
12 changes: 12 additions & 0 deletions packages/styled/__tests__/__snapshots__/styled.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,18 @@ exports[`styled objects 1`] = `
</h1>
`;

exports[`styled objects with dynamic value 1`] = `
.emotion-0 {
padding: 0.5rem;
}

<h1
className="emotion-0"
>
hello world
</h1>
`;

exports[`styled objects with spread properties 1`] = `
.emotion-0 {
font-size: 20px;
Expand Down
7 changes: 7 additions & 0 deletions packages/styled/__tests__/styled.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,13 @@ describe('styled', () => {
expect(tree).toMatchSnapshot()
})

test('objects with dynamic value', () => {
const H1 = styled('h1')({ padding: props => props.padding || '1rem' })
const tree = renderer.create(<H1 padding="0.5rem">hello world</H1>).toJSON()

expect(tree).toMatchSnapshot()
})

test('composing components', () => {
const Button = styled.button`
color: green;
Expand Down