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.

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

Emotion 10 greatly increases number of types during TypeScript compilation #1496

Closed
JakeGinnivan opened this issue Sep 11, 2019 · 18 comments
Closed

Comments

@JakeGinnivan
Copy link
Contributor

JakeGinnivan commented Sep 11, 2019

I am opening this to start a discussion, I plan on debugging it and trying to figure out why but upgrading to emotion 10 increased our project compilation time from ~30 seconds to ~70seconds. Any ideas/help would be great though.

Got these stats by just running yarn tsc --diagnostics

Current behavior:
Emotion 9:

Files:          2237
Lines:        308745
Nodes:       1049322
Identifiers:  331530
Symbols:     1031128
Types:        232302
Memory used: 750896K
I/O read:      1.47s
I/O write:     0.00s
Parse time:    5.07s
Bind time:     1.42s
Check time:   22.07s
Emit time:     0.00s
Total time:   28.55s

Emotion 10:

Files:           2246
Lines:         313024
Nodes:        1056772
Identifiers:   333322
Symbols:      1393179
Types:         852685
Memory used: 1185914K
I/O read:       1.94s
I/O write:      0.00s
Parse time:     5.67s
Bind time:      1.34s
Check time:    52.39s
Emit time:      0.00s
Total time:    59.40s

To reproduce:

I wanted to rule out our codebase, so have created a repo project at https://github.com/JakeGinnivan/emotion-types-sandbox

Emotion 10, with 1 styled component:
https://github.com/JakeGinnivan/emotion-types-sandbox

Files:           110
Lines:         84080
Nodes:        285703
Identifiers:   99654
Symbols:       67179
Types:         11015
Memory used: 115850K
I/O read:      0.08s
I/O write:     0.00s
Parse time:    0.97s
Bind time:     0.45s
Check time:    0.68s
Emit time:     0.00s
Total time:    2.09s

Emotion 9
https://github.com/JakeGinnivan/emotion-types-sandbox/tree/emotion9

Files:           108
Lines:         83784
Nodes:        284068
Identifiers:   99089
Symbols:       63864
Types:          2793
Memory used: 111337K
I/O read:      0.10s
I/O write:     0.00s
Parse time:    0.92s
Bind time:     0.35s
Check time:    0.18s
Emit time:     0.00s
Total time:    1.46s

Baseline (without emotion):

Files:           104
Lines:         83593
Nodes:        283283
Identifiers:   98776
Symbols:       62954
Types:          1206
Memory used: 110732K
I/O read:      0.12s
I/O write:     0.00s
Parse time:    0.92s
Bind time:     0.40s
Check time:    0.12s
Emit time:     0.00s
Total time:    1.43s

Styled components:

Files:           111
Lines:         93595
Nodes:        303502
Identifiers:  105234
Symbols:       71068
Types:          4978
Memory used: 125100K
I/O read:      0.14s
I/O write:     0.00s
Parse time:    1.01s
Bind time:     0.41s
Check time:    0.26s
Emit time:     0.00s
Total time:    1.68s

Summary:
Baseline (just react): 1206 types (1.43s @ 110,732K memory used)
Baseline + 1 emotion 9 styled component: 2793 types (1.46s @ 111,337K memory used)
Baseline + 1 emotion 10 styled component: 11,015 types (2.09s @ 115,850K memory used)
Baseline + 1 styled-components 4.3.2: 4978 types (1.68s @ 125,100K memory used)

Expected behavior:

The emotion TypeScript types need to be simplified

Environment information:

  • react version: 16.9.0
  • emotion version: 10.0.17
  • typescript version: 3.6.3
@Andarist
Copy link
Member

Could u also add comparison with styled-components?

@JakeGinnivan
Copy link
Contributor Author

Done, it is about double the number of types of emotion 9, but way less than emotion 10 still

@Andarist
Copy link
Member

Interesting. Maybe u could narrow down the explosion of types to some lines by bisecting our typings?

@JakeGinnivan
Copy link
Contributor Author

Yeah will be doing that, thought i'd just raise the issue for visibility first.

Another interesting data point:

Emotion 9, with 2 components: 2803 types (1.60s @ 110,600K)
Emotion 10, with 2 components: 11024 types (2.62s @ 132,442K)
Styled components with 2 components: 4987 types (2.07s @ 126,712K)

Not a huge amount of increase for any of the components. Gonna have to play around with themes and props too it seems (we use both of those heavily). Otherwise I am unsure how we got such a 600k type increase in our project.

@Andarist
Copy link
Member

Out of curiosity - when perf hits you? Do you have problem with initial compilation? recompilations? Do you use incremental builds feature?

@JakeGinnivan
Copy link
Contributor Author

We are using ts-loader so the PR's enabling incremental complication only merged in the last day. We often run webpack without type checking (the out of process stuff caused me all sorts of issues with client dev tools builds getting server chunks for some reason) then use tsc to type check.

I haven't setup the incremental builds feature yet, on the todo list. Was kinda waiting for ts-loader to ship support for incremental build

@JakeGinnivan
Copy link
Contributor Author

beta.4: Types: 6324
beta.6: Types: 11015

This is just the single component version. So beta 4 increased the types by quite a bit, but between beta 4 & 6 is when the large jump came in.

You can have a look at the changes in JakeGinnivan/emotion-types-sandbox@9612ac9

@JakeGinnivan
Copy link
Contributor Author

FYI @Ailrun @mitchellhamilton, any ideas welcome :)

@Andarist
Copy link
Member

That's the diff between beta.4 and beta.6 - 2a9a1b8...db5ccda

So looking at its contents this might be the offender:
2a9a1b8...db5ccda#diff-e68603b09cd6bb7bbc9c8690f15e5392R54

@JakeGinnivan
Copy link
Contributor Author

So in my sandbox project I have copied out the .d.ts files from the project at various versions so we can easily check performance of just types at a version.

JakeGinnivan/emotion-types-sandbox@9612ac9 lists the diff between beta.4 and beta.6 of just the types.

In @emotion/serialize/index.d.ts, changing
Equal<MP, undefined, never, FunctionInterpolation<MP>>
back to
(undefined extends MP ? never : FunctionInterpolation<MP>)

And getting rid of that Equal type

Drops the number of types from 11015 to 6840. That brings it back to close to beta.4 levels. Still not back to emotion 9 levels but much better.

@Andarist
Copy link
Member

Seems like this could be inlined, as both seem equivalent, OTOH emotion uses conditional types etc extensively - we just can't get rid all of them.

@JakeGinnivan
Copy link
Contributor Author

If we remove Equals entirely and remove the circular reference from FunctionInterpolation we dropped from 852,685 to 749,563 types.

export interface FunctionInterpolation<MP> {
  (mergedProps: MP):
    | null
    | undefined
    | boolean
    | number
    | string
    | ComponentSelector
    | Keyframes
    | SerializedStyles
    | ArrayInterpolation<MP>
    | ObjectInterpolation<MP>
}

export type Interpolation<MP = undefined> =
  | null
  | undefined
  | boolean
  | number
  | string
  | ComponentSelector
  | Keyframes
  | SerializedStyles
  | ArrayInterpolation<MP>
  | ObjectInterpolation<MP>
  | FunctionInterpolation<MP>

Working on trying to simplify the CreateStyled types

@JakeGinnivan
Copy link
Contributor Author

Ok, have restructured a bunch of types. I've moved theme related overloads into emotion-theming and removed a bunch of conditional types.

https://github.com/JakeGinnivan/emotion-types-sandbox/pull/1/files

Here are the diagnostics in our application. The number of types are better than they were for emotion 9, same with memory. Time is up because i'm working on battery not plugged in i'm gathering.

Files: 2242
Lines: 282748
Nodes: 1012482
Identifiers: 322586
Symbols: 1316681
Types: 230072
Memory used: 926197K
I/O read: 1.98s
I/O write: 0.00s
Parse time: 5.47s
Bind time: 1.09s
Check time: 40.04s
Emit time: 0.00s
Total time: 46.60s

Should I put together a PR to emotion?

@JakeGinnivan
Copy link
Contributor Author

It also fixed a few TODOs and type inference issues

@Andarist
Copy link
Member

Should I put together a PR to emotion?

Sure, I'd love to take a look at that.

@JakeGinnivan
Copy link
Contributor Author

There you go, works in our project really well. It does have breaking changes in the types. Not sure how you semver your types.

@orta
Copy link

orta commented Sep 13, 2019

I've started looking at a way to be able to catch these issues early with existing dts + js projects also: tsdjs/tsd#34

@Andarist
Copy link
Member

Andarist commented Nov 3, 2019

Going to close this issue right now as we plan to land the PR intended to improve this soon and I'd like to keep the issue tracker clean as I'm going through the list daily.

@Andarist Andarist closed this as completed Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants