Jss: Upgrading jss and react-jss from 10.0.2 to 10.0.4 breaks TypeScript compile step.

Created on 31 Jan 2020  Â·  11Comments  Â·  Source: cssinjs/jss

We're currently trying to upgrade our React-based design system thenativeweb-ux which uses JSS and react-jss. We'd like to upgrade from 10.0.2 to 10.0.4. Unfortunately this upgrade breaks the TypeScript compile step.

__Expected behavior:__

Compiling a React- and TypeScript-based codebase should run in a reasonable amount of time.

__Describe the bug:__

With earlier versions (e.g. 10.0.0, 10.0.1, 10.0.2) the TypeScript performance was solid. The upgrade to version 10.0.4 caused the compliation process to hang until it completely crashes the Node.js process since it's running out of memory. See the the logs of the Github action runs the compilation step (The task is called 'Run roboter').

__Codesandbox link:__

The whole repo is Open Source, so we can share all the code and CI logs. Here is the PR that performs the upgrade. The master branch is still on 10.0.2 and happily compiling. Again here are the logs of the Github action that runs the compile step. It's possible to clone the repo locally, run npm install, and then npx roboter build or npx tsc.

__Versions:__

  • jss: 10.0.4
  • OS: macOS and Linux

__Insights so far:__

  • The actual type checking seems to work. When you run npx tsc --noEmit --diagnostics (which will only type check but not output any files) the process completes in 7-8 seconds.
  • We also did some tests with 10.0.3 which also caused this performance. So it seems like something introduced in 10.0.3 might be the cause of this issue.

Maybe somebody else is experiencing issues like this? Anyway, we're grateful for any hints into what might be the cause for this behaviour.

typescript

Most helpful comment

I had another look at the defintion file to better identify the bottleneck. I have some additional insights I wanted to share.

My first guess was that the bottleneck happens because of this intersection of types:

type JssStyleP<S> = CssProperties & {
  [key: string]: FnValue<JssValue | S>
}

However this is not the actual root of the problem. Because you can change this definition to:

type JssStyleP<S> = NormalCssProperties & {
  [key: string]: FnValue<JssValue | S>
}

…and the bottleneck is gone. That lead me to the question: what is the difference between the two types CssProperties and NormalCssProperties? Here's what they look like:

type NormalCssProperties = css.Properties<string | number>
type CssProperties = {[K in keyof NormalCssProperties]: FnValue<NormalCssProperties[K]>}

So CssProperties is a mapped type which seems like is needed to set dynamic values in a style definition. In this mapped type we're mapping over all the keys of NormalCssProperties. If you grap the keys by hand like this…

type KeysOfNormalCssProperties = keyof NormalCssProperties;

… you see that NormalCssProperties has about 750 keys.

keys-of-css-properties

This does not seem much but it's enough to completely slow down the compile step. So the problem here is: We're creating a mapped type that maps over 750 keys. Unfortunately I haven't found a solution yet on how to avoid or prevent this from happening. But hopefully this information is helpful to others.

All 11 comments

+1: repeat in our production

I hade some time today to dig a bit deeper into the problem. I tried to make small changes inside the main definiton file of the JSS package in order to track down a possible bottle neck.

I also found other TypeScript issues, e.g. this one that originated in the styled-components ecoystem that also reported slow TypeScript compliation times. In this particular thread there's a comment pointing out that

... performance impact is caused by a large union of types (eg, the union of all possible jsx components)...

So scanned the definition file for possibly large unions of types and found this line. So when I change this line from…

// Jss Style definitions
type JssStyleP<S> = CssProperties & {
  [key: string]: FnValue<JssValue | S>
}

…to…

// Jss Style definitions
type JssStyleP<S> =  {
  [key: string]: FnValue<JssValue | S>
}

… the bottleneck is gone and compilation time is back to normal. Also the code editor is responsive again and is able to report TypeScript issues again.

@kof Is there any chance to get this change into a (soon to be released) patch update?

Small update: we've upgraded TypeScript to 3.7.2. It seems like memory is handled better by TypeScript now but the performance bottleneck still exists. The compilation now completes but after running for about 40 minutes on CI. With the previous TypeScript version compilation crashed on CI after 20 minutes while running out of memory. So this still makes 10.0.4 unusable for us. 😞

After removing the CssProperties & locally from the index.d.ts file as described above, the build completes in about 20 seconds.

@yeliex Could you resolve the problem in your build?

Is there anything we can do to fix this?

@yeliex Maybe, if you have the time, if would be interesting to see whether the fix suggested by @mattwagl also resolves your issues. Could you maybe verify this?

I had another look at the defintion file to better identify the bottleneck. I have some additional insights I wanted to share.

My first guess was that the bottleneck happens because of this intersection of types:

type JssStyleP<S> = CssProperties & {
  [key: string]: FnValue<JssValue | S>
}

However this is not the actual root of the problem. Because you can change this definition to:

type JssStyleP<S> = NormalCssProperties & {
  [key: string]: FnValue<JssValue | S>
}

…and the bottleneck is gone. That lead me to the question: what is the difference between the two types CssProperties and NormalCssProperties? Here's what they look like:

type NormalCssProperties = css.Properties<string | number>
type CssProperties = {[K in keyof NormalCssProperties]: FnValue<NormalCssProperties[K]>}

So CssProperties is a mapped type which seems like is needed to set dynamic values in a style definition. In this mapped type we're mapping over all the keys of NormalCssProperties. If you grap the keys by hand like this…

type KeysOfNormalCssProperties = keyof NormalCssProperties;

… you see that NormalCssProperties has about 750 keys.

keys-of-css-properties

This does not seem much but it's enough to completely slow down the compile step. So the problem here is: We're creating a mapped type that maps over 750 keys. Unfortunately I haven't found a solution yet on how to avoid or prevent this from happening. But hopefully this information is helpful to others.

How was this different in 10.0.2? Have there been any relevant changes in this area?

I had a look at the commit history and this PR introduced the change in 10.0.3.

If I remove the mapped type and adjust the definition of JssStyleP like this…

type JssStyleP<S> = css.Properties<string | number | ((data: any) => string | number)> & {
  [key: string]: FnValue<JssValue | S>
}

…compilation works again.

@henribeck: Can I create a PR for this? Or are there reasons for sticking to the mapped type approach? Thanks for your help!

I wanted to give an update and document our findings for anybody experiencing similiar performance issues with TypeScript and JSS because we've managed to get the compile time back to a usable level.

We could trace the problem to two sources:

  • A misconfigured tsconfig.json. We use Next.js as our application framework and we have two applications living in our design system repository (a styleguide and a test application we use for running integration tests). In both application folders Next.js generates a hidden .next and a dist folder. Both folders can contain large JS and TS files. So these folder of compiled code are actually one source that slowing down TypeScript. We explicitly had to ignore the files inside these directories in order to improve performance speed. This is our current root level tsconfig that exludes these files explicitly. Still not sure why this did not cause problemes before 10.0.2 but this was the largest bottleneck for us.
  • Still the mapped type I mentioned above (CssProperties) seems to be a bottleneck as well. So before we used the Styles type from the jss module to type nested style objects. This type uses the mapped type under the hood. So we thought about why this mapped type is nedded at all: dynamic style props seem to be the reason why this mapped type exists. So in our use case we actually don't use use dynamic props. We only use themed style functions that we pass to createUseStyles. We chose to create a local type defintion that works for us.

So far this setup works well and we can now enjoy strongly types style objects. Thanks @kof for the patience and for creating JSS. I'm going to close this issue now but hopefully these findings can be useful to others.

@mattwagl great write up! Do you see any way for react-jss to improve perf of those types while still having support for the full syntax?

@kof I haven't found another way to generate a type like CssPropertiers without mapping accross all the properties provided by the csstype in order to change the value to a dynamic function. Maybe some of the TypeScript experts have some additional ideas.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

glowkeeper picture glowkeeper  Â·  5Comments

janhartmann picture janhartmann  Â·  5Comments

HenriBeck picture HenriBeck  Â·  4Comments

sergiop picture sergiop  Â·  5Comments

EugeneSnihovsky picture EugeneSnihovsky  Â·  4Comments