Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reorganize @emotion/utils types #3076

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reorganize @emotion/utils types #3076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/six-hotels-double.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@emotion/cache': minor
'@emotion/css': minor
'@emotion/react': minor
'@emotion/serialize': minor
'@emotion/server': minor
'@emotion/utils': minor
---

Reorganize @emotion/utils types
28 changes: 26 additions & 2 deletions packages/cache/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,32 @@
// Definitions by: Junyoung Clare Jang <https://github.com/Ailrun>
// TypeScript Version: 2.2
import { EmotionCache } from '@emotion/utils'
import { SerializedStyles } from '@emotion/serialize'
import { StyleSheet } from '@emotion/sheet'

export { EmotionCache }
export interface RegisteredCache {
[key: string]: string
}

export interface EmotionStyleSheet extends StyleSheet {
constructor: typeof StyleSheet
}

export interface EmotionCache {
inserted: {
[key: string]: string | true
}
registered: RegisteredCache
sheet: EmotionStyleSheet
Copy link
Author

Choose a reason for hiding this comment

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

This is the only thing I can think of that would solve the constructor issue, let me know if you don't want it or if you have any thoughts

key: string
compat?: true
nonce?: string
insert(
selector: string,
serialized: SerializedStyles,
sheet: StyleSheet,
shouldCache: boolean
): string | void
}

export interface StylisElement {
type: string
Expand Down
8 changes: 7 additions & 1 deletion packages/cache/types/tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,10 @@ declare const testOptions: Options
// $ExpectType EmotionCache
createCache({ key: 'test-key' })
// $ExpectType EmotionCache
createCache(testOptions)
const cache = createCache(testOptions)

// $ExpectType StyleSheet
new cache.sheet.constructor({
key: 'css',
container: document.createElement('div')
})
15 changes: 15 additions & 0 deletions packages/react/types/tests-css.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@ css`
top: ${'20px'};
`

// $ExpectType SerializedStyles
css([{ display: null }])

// $ExpectType SerializedStyles
css({
':hover': [{ color: 'green' }, { backgroundColor: 'yellow' }]
})

// $ExpectType SerializedStyles
css({
':hover': css`
color: hotpink;
`
})

// $ExpectError
css(() => 'height: 300px;')
Copy link
Author

Choose a reason for hiding this comment

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

@Andarist Are there reasons why this and below it are not allowed even though they worked?

Copy link
Member

Choose a reason for hiding this comment

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

this css call doesn't run in any kind of "context", we don't have any arguments to give to this function so we don't allow functions to be passed here

Copy link
Author

Choose a reason for hiding this comment

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

So I'm guessing, even though it works in nature, there's no point in doing this as it doesn't run in any context?
I'm looking for reasons why we do not allow these things in TS due to type complaining, but it's working just fine.


Expand Down
15 changes: 11 additions & 4 deletions packages/serialize/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
// Definitions by: Junyoung Clare Jang <https://github.com/Ailrun>
// TypeScript Version: 2.8

import { RegisteredCache, SerializedStyles } from '@emotion/utils'
import type { RegisteredCache } from '@emotion/cache'
import * as CSS from 'csstype'

export { RegisteredCache, SerializedStyles }
export interface SerializedStyles {
name: string
styles: string
map?: string
next?: SerializedStyles
}

export type CSSProperties = CSS.PropertiesFallback<number | string>
export type CSSPropertiesWithMultiValues = {
[K in keyof CSSProperties]:
| null
| undefined
| false
Comment on lines +16 to +18
Copy link
Member

Choose a reason for hiding this comment

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

So I guess those were added to account for "a test that allows false value but not in TS"?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but a weird thing happened, I managed to build my own serializer based on the implementation but using js-beautify like beautify.css() instead of using @emotion/css-prettifier.

When I used it and passed null like css({ textAlign: null }), it created a weird selector like .emotion-0 textAlign {} but when I switched to @emotion/css-prettifier, it removed it.

The @emotion/css-prettifier also removes the empty selector like .emotion-0 {} which I guess does the same thing hence why in the test we're not seeing these weird things.

It's not an issue as I can just use that lib you guys already have but just a weird thing that it only happens when it's null

| CSSProperties[K]
| Array<Extract<CSSProperties[K], string>>
}

export type CSSPseudos = { [K in CSS.Pseudos]?: CSSObject }
export type CSSPseudos = { [K in CSS.Pseudos]?: CSSInterpolation }
Copy link
Author

Choose a reason for hiding this comment

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

  • a test that allows CSS composition but not in TS
  • a test that allows false value but not in TS
  • a test that allows child selector array but not in TS

Copy link
Member

Choose a reason for hiding this comment

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

Looks legit but I will have to reexamine this more closely later.


export interface ArrayCSSInterpolation extends Array<CSSInterpolation> {}

Expand Down
2 changes: 1 addition & 1 deletion packages/server/types/create-instance.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// TypeScript Version: 2.8

/// <reference types="node" />
import { EmotionCache } from '@emotion/utils'
import { EmotionCache } from '@emotion/cache'

export interface EmotionCritical {
html: string
Expand Down
2 changes: 1 addition & 1 deletion packages/server/types/tests-create-instance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import createEmotionServer from '@emotion/server/create-instance'
import { EmotionCache } from '@emotion/utils'
import { EmotionCache } from '@emotion/cache'

declare const cache: EmotionCache

Expand Down
19 changes: 14 additions & 5 deletions packages/utils/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
// Definitions by: Junyoung Clare Jang <https://github.com/Ailrun>
// TypeScript Version: 2.2
import type { SerializedStyles as EmotionSerializedStyles } from '@emotion/serialize'
import type {
RegisteredCache as EmotionRegisteredCache,
EmotionCache as EmotionStyleCache
} from '@emotion/cache'

/** @deprecated use `RegisteredCache` from `@emotion/cache` */
export interface RegisteredCache {
[key: string]: string
}

/** @deprecated use `StyleSheet` from `@emotion/sheet` */
export interface StyleSheet {
container: HTMLElement
nonce?: string
Expand All @@ -14,6 +21,7 @@ export interface StyleSheet {
tags: Array<HTMLStyleElement>
}

/** @deprecated use `EmotionCache` from `@emotion/cache` */
export interface EmotionCache {
inserted: {
[key: string]: string | true
Expand All @@ -31,6 +39,7 @@ export interface EmotionCache {
): string | void
}

/** @deprecated use `SerializedStyles` from `@emotion/serialize` */
export interface SerializedStyles {
name: string
styles: string
Expand All @@ -41,19 +50,19 @@ export interface SerializedStyles {
export const isBrowser: boolean

export function getRegisteredStyles(
registered: RegisteredCache,
registered: EmotionRegisteredCache,
registeredStyles: Array<string>,
classNames: string
): string

export function registerStyles(
cache: EmotionCache,
serialized: SerializedStyles,
cache: EmotionStyleCache,
serialized: EmotionSerializedStyles,
isStringTag: boolean
): void

export function insertStyles(
cache: EmotionCache,
serialized: SerializedStyles,
cache: EmotionStyleCache,
serialized: EmotionSerializedStyles,
Copy link
Author

Choose a reason for hiding this comment

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

@Andarist This is what I ended up doing otherwise, they would be incompatible to each other.

isStringTag: boolean
): string | void
11 changes: 2 additions & 9 deletions packages/utils/types/tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import {
EmotionCache,
RegisteredCache,
SerializedStyles,
StyleSheet,
getRegisteredStyles,
Andarist marked this conversation as resolved.
Show resolved Hide resolved
insertStyles,
isBrowser
} from '@emotion/utils'
import type { EmotionCache, RegisteredCache } from '@emotion/cache'
import { getRegisteredStyles, insertStyles, isBrowser } from '@emotion/utils'

declare const testCache: EmotionCache
declare const testRegisteredCache: RegisteredCache
Expand Down
3 changes: 2 additions & 1 deletion packages/utils/types/tslint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"extends": "@definitelytyped/dtslint/dtslint.json",
"rules": {
"array-type": [true, "generic"],
"semicolon": false
"semicolon": false,
"no-redundant-jsdoc": false
Copy link
Author

Choose a reason for hiding this comment

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

@Andarist I tried all of these options but didn't work so I had to do this instead https://palantir.github.io/tslint/usage/rule-flags/

}
}