Convert `@emotion/primitives` to TypeScript #2849

srmagura · 2022-08-07T14:09:52Z

Part of #2358.

I'm not super confident this is correct, but 🤷‍♀️. This is mainly in reference to the one comment I about to leave on the PR diff.

This also removes some .d.ts files that got brought into ts-migration when I merged main yesterday.

CI is going to fail on this because of the Emotion website — I addressed this issue in a separate PR.

changeset-bot · 2022-08-07T14:09:55Z

🦋 Changeset detected

Latest commit: 03a4dcd

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

This PR includes changesets to release 1 package

Name	Type
@emotion/primitives	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

srmagura · 2022-08-07T14:11:06Z

packages/primitives-core/src/types.ts

 export interface AbstractStyleSheet {
  create<T extends NamedStyles<T> | NamedStyles<any>>(
    styles: T | NamedStyles<T>
-  ): T
+  ): { [P in keyof T]: number }


@types/react-native has the return type as T which seems wrong to me, and it was causing issues... This is the thing I am not totally sure is correct.

codesandbox-ci · 2022-08-07T14:11:40Z

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 03a4dcd:

Sandbox	Source
Emotion	Configuration

codecov · 2022-08-07T14:14:40Z

Codecov Report

Merging #2849 (03a4dcd) into ts-migration (993557d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
packages/primitives-core/src/styled.ts	`100.00% <ø> (ø)`
packages/primitives-core/src/css.ts	`100.00% <100.00%> (ø)`
packages/primitives/src/index.ts	`100.00% <100.00%> (ø)`
packages/primitives/src/styled.ts	`100.00% <100.00%> (ø)`
packages/primitives/src/test-props.ts	`100.00% <100.00%> (ø)`

Andarist · 2022-08-16T22:48:17Z

packages/primitives/test/emotion-primitives.test.tsx

    expect(() => styled.TEXT({})).toThrow()
  })

  test('should render the primitive when styles applied using object style notation', () => {
    const Text = styled.Text`
      color: red;
      font-size: 20px;
-      background-color: ${props => props.back};
+      background-color: ${(props: any) => props.back};


could we declare those props using generics rather than using any?

styled.Text does not currently take any generic parameters. Are you suggesting we change the types so that it behaves similarly to styled from @emotion/styled? Meaning that on this line of code, TypeScript would know that props has type TextProps.

I believe that goes beyond just converting the package to TypeScript since it would mean changing user-facing types, but we can do that if you want to.

styled.Text does not currently take any generic parameters. Are you suggesting we change the types so that it behaves similarly to styled from @emotion/styled? Meaning that on this line of code, TypeScript would know that props has type TextProps.

That would be ideal. It might not be the easiest task though - so let me know if you prefer not to do it right now.

I believe that goes beyond just converting the package to TypeScript since it would mean changing user-facing types, but we can do that if you want to.

Are there any user-facing types here though? I can't find any.

packages/primitives/src/styled.ts

Andarist · 2022-08-17T08:11:23Z

packages/primitives-core/src/types.ts


-  flatten(style?: unknown[]): unknown
+  flatten<T>(style?: StyleProp<T>): T extends (infer U)[] ? U : T


Conceptually this might not be enough because flatten accepts nested arrays, so this should be recursive.

I wonder though if this is even needed. It feels a little bit like maybe some kind of a style prop should be generic.

We already have not quite an accurate return type for the create method - in React Native Stylesheet.create returns the input object, it's basically an identity function:
https://github.com/facebook/react-native/blob/ba383baf357bc6def94280d28f69efbab341fe36/Libraries/StyleSheet/StyleSheet.js#L371

What I'm getting at is that we personally don't quite care about all of those types internally - we should just treat them as close to unknown as possible. We only need to make sure that given methods are available on the input StyleSheet in places like here:

emotion/packages/native/src/base.js

Line 7 in 993557d

let styled = createStyled(StyleSheet)

Maybe we should just make a StyleSheet itself a generic? I'm not entirely sure about this but perhaps the best way to figure this out would be to convert this one together with @emotion/native. I feel like it's hard to figure out those types in isolation right now because the final design needs to be dictated by the needs of both @emotion/primitives and @emotion/native.

An offtopic note - I wish we didn't have those libraries at all 😅

We already have not quite an accurate return type for the create method - in React Native Stylesheet.create returns the input object, it's basically an identity function:

I could have sworn I tested StyleSheet.create and it returned an object whose values were numbers... I think they might have changed that in a semi-recent version of React Native. Anyway, I have corrected the create return type so that it matches @types/react-native.

I feel like it's hard to figure out those types in isolation right now because the final design needs to be dictated by the needs of both @emotion/primitives and @emotion/native.

Yes, it is hard to figure them out in isolation. So do you think I should convert @emotion/native in this PR?

An offtopic note - I wish we didn't have those libraries at all 😅

Totally agree LOL, I don't see how Emotion would be beneficial when using React Native...

Yes, it is hard to figure them out in isolation. So do you think I should convert @emotion/native in this PR?

Yeah, alternatively - you could branch off this PR and prepare @emotion/native migration as a separate PR that would target this PR instead of ts-migration branch. This way we gonna be able to land and review both separately but at the same time, those types here wont be developed in isolation.

srmagura · 2022-08-22T13:31:28Z

Good points @Andarist . I am going to...

~~Attempt to make the @emotion/primitives(-core) styled function accept the props type as a generic.~~ Edit: Spent some time on this and it was way harder than I thought. Part of the issue is that @emotion/primitives defines styled.View and styled.Text but ViewProps and TextProps are from @types/react-native. This is an issue because @emotion/primitives is not supposed to be tied to React Native specifically...
Branch a new PR off of this one to convert @emotion/native

srmagura · 2022-08-22T15:28:23Z

@Andarist Just converted @emotion/native here: #2861. No changes were required to primitives or primitives-core so this PR should be ready for merge.

srmagura requested a review from Andarist August 7, 2022 14:09

srmagura commented Aug 7, 2022

View reviewed changes

Andarist reviewed Aug 16, 2022

View reviewed changes

packages/primitives/src/styled.ts Show resolved Hide resolved

Andarist reviewed Aug 17, 2022

View reviewed changes

srmagura requested a review from Andarist August 18, 2022 15:07

srmagura added 6 commits August 18, 2022 13:36

Convert primitives package to TypeScript

acdb81c

Convert @emotion/primitives tests to TypeScript

588688e

Add changeset, add "types" to package.json

8685d3f

Remove old snapshots

4d1ef69

@emotion/primitives TypeScript tweaks

8946f62

Change something to any to fix typescript errors

03a4dcd

srmagura force-pushed the ts-migration-primitives branch from 3189b0c to 03a4dcd Compare August 18, 2022 17:44

srmagura marked this pull request as draft August 22, 2022 13:32

srmagura mentioned this pull request Aug 22, 2022

Migrate @emotion/native to TypeScript #2861

Closed

srmagura marked this pull request as ready for review August 22, 2022 15:28

Andarist force-pushed the ts-migration branch 2 times, most recently from 04c9f33 to 1effaf9 Compare May 11, 2024 11:59

emmatown deleted the branch emotion-js:ts-migration May 17, 2024 18:43

emmatown closed this May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert `@emotion/primitives` to TypeScript #2849

Convert `@emotion/primitives` to TypeScript #2849

srmagura commented Aug 7, 2022

changeset-bot bot commented Aug 7, 2022 •

edited

Loading

srmagura Aug 7, 2022

codesandbox-ci bot commented Aug 7, 2022 •

edited

Loading

codecov bot commented Aug 7, 2022 •

edited

Loading

Andarist Aug 16, 2022

srmagura Aug 18, 2022

Andarist Aug 22, 2022

Andarist Aug 17, 2022

srmagura Aug 18, 2022

Andarist Aug 22, 2022

srmagura commented Aug 22, 2022 •

edited

Loading

srmagura commented Aug 22, 2022


		flatten(style?: unknown[]): unknown
		flatten<T>(style?: StyleProp<T>): T extends (infer U)[] ? U : T

Convert @emotion/primitives to TypeScript #2849

Convert @emotion/primitives to TypeScript #2849

Conversation

srmagura commented Aug 7, 2022

changeset-bot bot commented Aug 7, 2022 • edited Loading

🦋 Changeset detected

srmagura Aug 7, 2022

Choose a reason for hiding this comment

codesandbox-ci bot commented Aug 7, 2022 • edited Loading

codecov bot commented Aug 7, 2022 • edited Loading

Codecov Report

Andarist Aug 16, 2022

Choose a reason for hiding this comment

srmagura Aug 18, 2022

Choose a reason for hiding this comment

Andarist Aug 22, 2022

Choose a reason for hiding this comment

Andarist Aug 17, 2022

Choose a reason for hiding this comment

srmagura Aug 18, 2022

Choose a reason for hiding this comment

Andarist Aug 22, 2022

Choose a reason for hiding this comment

srmagura commented Aug 22, 2022 • edited Loading

srmagura commented Aug 22, 2022

Convert `@emotion/primitives` to TypeScript #2849

Convert `@emotion/primitives` to TypeScript #2849

changeset-bot bot commented Aug 7, 2022 •

edited

Loading

codesandbox-ci bot commented Aug 7, 2022 •

edited

Loading

codecov bot commented Aug 7, 2022 •

edited

Loading

srmagura commented Aug 22, 2022 •

edited

Loading