Reorganize `@emotion/utils` types #3076

JeromeDeLeon · 2023-07-15T07:29:30Z

What: #3075

Why: To fix inconsistent type from #3075

How: Reorganizing the @emotion/utils by moving the types to the proper package.

Checklist:

Documentation n/a
Tests
Code complete - n/a
Changeset

I'm not able to fix the constructor issue. When I tried to convert it to interface /types, it complains as it was either being merged to StyleSheet DOM type or a duplicate name.

changeset-bot · 2023-07-15T07:29:33Z

🦋 Changeset detected

Latest commit: 67518ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@emotion/cache	Minor
@emotion/css	Minor
@emotion/react	Minor
@emotion/serialize	Minor
@emotion/server	Minor
@emotion/utils	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci · 2023-07-15T07:31:27Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 67518ed:

Sandbox	Source
Emotion	Configuration

JeromeDeLeon · 2023-07-15T07:31:34Z

~~@Andarist I ended up touching some .js files but flow complains about the type not being declared and has no idea how to fix them 😞~~

I realized that in flow, you must declare types in .js, hence why I saw repeated types both in .js and .d.ts. Once I reverted, everything went fine now.

packages/cache/src/index.js

packages/utils/src/types.js

packages/utils/types/tests.ts

JeromeDeLeon · 2023-07-15T08:14:19Z

packages/cache/types/index.d.ts

+    [key: string]: string | true
+  }
+  registered: RegisteredCache
+  sheet: EmotionStyleSheet


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

packages/css/src/create-instance.js

JeromeDeLeon · 2023-07-15T08:16:27Z

~~Noob question, how would I be able to test this on my package locally? I just want to make sure that everything I did here works 😅~~

~~The test seems to tell me that everything is fine~~

I downloaded the build via https://pkg.csb.dev/emotion-js/emotion/commit/1776c402/@emotion/* and managed to remove all types issues

JeromeDeLeon · 2023-07-21T11:28:34Z

packages/react/types/tests-css.tsx

+  ':hover': css`
+    color: hotpink;
+  `
+})

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


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

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

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.

JeromeDeLeon · 2023-07-22T10:09:25Z

packages/serialize/types/index.d.ts

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

-export type CSSPseudos = { [K in CSS.Pseudos]?: CSSObject }
+export type CSSPseudos = { [K in CSS.Pseudos]?: CSSInterpolation }


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

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

Andarist · 2023-07-24T21:32:07Z

packages/serialize/types/index.d.ts

+    | null
+    | undefined
+    | false


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

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

Andarist · 2023-07-24T21:35:58Z

packages/utils/types/index.d.ts

+import type { SerializedStyles } from '@emotion/serialize'
+import type { RegisteredCache, EmotionCache } from '@emotion/cache'


We can't import from other packages here because we don't depend on them. This is also true at type level - even if we still don't import anything at runtime.

I'd keep the existing types here (to avoid breaking changes) but I'd deprecate them in favor of the proper ones exported by specific packages). In the future, we should just remove export from those local types here. This way we'll be able to depend on TS structural type system without introducing any dependencies to this package.

JeromeDeLeon · 2023-07-25T04:29:43Z

packages/utils/types/index.d.ts

-  cache: EmotionCache,
-  serialized: SerializedStyles,
+  cache: EmotionStyleCache,
+  serialized: EmotionSerializedStyles,


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

JeromeDeLeon · 2023-07-25T04:31:05Z

packages/utils/types/tslint.json

@@ -2,6 +2,7 @@
  "extends": "@definitelytyped/dtslint/dtslint.json",
  "rules": {
    "array-type": [true, "generic"],
-    "semicolon": false
+    "semicolon": false,
+    "no-redundant-jsdoc": false


@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/

JeromeDeLeon · 2023-08-08T00:04:52Z

@Andarist Is there anything blocking this PR? I'm happy to address them 😄

JeromeDeLeon commented Jul 15, 2023

View reviewed changes

packages/cache/src/index.js Outdated Show resolved Hide resolved

JeromeDeLeon commented Jul 15, 2023

View reviewed changes

packages/utils/src/types.js Outdated Show resolved Hide resolved

JeromeDeLeon commented Jul 15, 2023

View reviewed changes

packages/utils/types/tests.ts Show resolved Hide resolved

JeromeDeLeon force-pushed the chore/reorganize-types branch 2 times, most recently from 0b9529c to bf56fb8 Compare July 15, 2023 08:08

JeromeDeLeon commented Jul 15, 2023

View reviewed changes

packages/css/src/create-instance.js Outdated Show resolved Hide resolved

JeromeDeLeon force-pushed the chore/reorganize-types branch from bf56fb8 to 1776c40 Compare July 21, 2023 10:59

JeromeDeLeon commented Jul 21, 2023

View reviewed changes

JeromeDeLeon force-pushed the chore/reorganize-types branch from 1776c40 to 361296c Compare July 22, 2023 10:05

JeromeDeLeon commented Jul 22, 2023

View reviewed changes

Andarist reviewed Jul 24, 2023

View reviewed changes

reorganize @emotion/utils types

67518ed

JeromeDeLeon force-pushed the chore/reorganize-types branch from 361296c to 67518ed Compare July 25, 2023 04:28

JeromeDeLeon commented Jul 25, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize `@emotion/utils` types #3076

Reorganize `@emotion/utils` types #3076

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

changeset-bot bot commented Jul 15, 2023 •

edited

Loading

codesandbox-ci bot commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon Jul 15, 2023

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon Jul 21, 2023

Andarist Jul 24, 2023

JeromeDeLeon Jul 25, 2023

JeromeDeLeon Jul 22, 2023

Andarist Jul 24, 2023

Andarist Jul 24, 2023

JeromeDeLeon Jul 25, 2023

Andarist Jul 24, 2023

JeromeDeLeon Jul 25, 2023

JeromeDeLeon Jul 25, 2023 •

edited

Loading

JeromeDeLeon commented Aug 8, 2023

		import type { SerializedStyles } from '@emotion/serialize'
		import type { RegisteredCache, EmotionCache } from '@emotion/cache'

Reorganize @emotion/utils types #3076

Are you sure you want to change the base?

Reorganize @emotion/utils types #3076

Conversation

JeromeDeLeon commented Jul 15, 2023 • edited Loading

changeset-bot bot commented Jul 15, 2023 • edited Loading

🦋 Changeset detected

codesandbox-ci bot commented Jul 15, 2023 • edited Loading

JeromeDeLeon commented Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

JeromeDeLeon commented Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeDeLeon Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

JeromeDeLeon commented Aug 8, 2023

Reorganize `@emotion/utils` types #3076

Reorganize `@emotion/utils` types #3076

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

changeset-bot bot commented Jul 15, 2023 •

edited

Loading

codesandbox-ci bot commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon commented Jul 15, 2023 •

edited

Loading

JeromeDeLeon Jul 25, 2023 •

edited

Loading