SSR memory leak with xstyled 3.x.x

[StephaneRob]

🐛 Bug Report

We recently faced a memory leak on our SSR application after upgrading xstyled from 2.5.0 to 3.6.0. Here are two screenshots of the memory breakdown when we released the upgrade of xstyled and the rollback.

We detected a heap size continuously growing on reduceVariants function.

[jguddas]

My guess would be that this is due to the cache continuously growing.

[gregberge]

Hello @StephaneRob, thanks for pointing it out. I will dig it to understand the problem. Feel free to reach me on Twitter if you have more information.

[EmilioAiolfi]

@gregberge Folks some news about this? I think using stitches or Styled components + Xstyled, but SSR is very import for my project.

[mleralec]

Hey @gregberge, I'm trying to solve this issue, can you look at my PR https://github.com/gregberge/xstyled/pull/379 ? (it will be useful for debug 👀 )

[jguddas]

I was able to reproduce this with Next.JS, the way to fix it for me here was like this:

 import { x, ThemeProvider } from '@xstyled/styled-components'

+ const theme = {}
+ 
 export default function Home() {
   return (
-    <ThemeProvider theme={{}}>
+    <ThemeProvider theme={theme}>
        <x.div color="red">Hello World</x.div>
     </ThemeProvider>
   )
 }

Instead of a WeakMap here we could use a Map and on cache.set do something like this:

const MAX_CACHE_SIZE = 300
if (cache.size > MAX_CACHE_SIZE) {
  if (__DEV__) {
    console.warn('Your theme might not be getting cached properly! https://github.com/gregberge/xstyled/issues/371')
  }
  return null
}

This would stop caching after 300 themes and print a warning in webpack dev environments (this includes next 12+) when setup correctly with rollup and exports.development in the package.json.

---

We could also use something like quick-lru.

[mleralec]

Hey @jguddas, thanks for your answer. What do you think of something like that ?

- const cacheSupported: boolean = typeof Map !== 'undefined' && typeof WeakMap !== 'undefined'
+ const cacheSupported: boolean = typeof Map !== 'undefined'

- const caches = cacheSupported ? new WeakMap<ITheme, ThemeCache>() : null
+ const caches = cacheSupported ? new Map<string, ThemeCache>() : null
+ const stringifyTheme = (theme: ITheme): string => JSON.stringify(theme)

const getThemeCache = (theme: ITheme): ThemeCache | null => {
   if (caches === null) return null
+  const stringifiedTheme = stringifyTheme(theme)
-  if (caches.has(theme)) return caches.get(theme) || null
+  if (caches.has(stringifiedTheme)) return caches.get(stringifiedTheme) || null
   const cache = {}
-  caches.set(theme, cache)
+  caches.set(stringifiedTheme, cache)
   return cache
}

If the problem is related to the WeakMap, we can stringify the Theme and use a classic Map (JSON.stringify is used here, but we can use something to hash an object like https://www.npmjs.com/package/object-hash)

[jguddas]

@mleralec, this will have a significant performance impact, but we could run it in dev to throw an error like this:

+ const cacheValidation = cacheSupported ? new Map<string, true>() : null
+ 
const getThemeCache = (theme: ITheme): ThemeCache | null => {
  if (caches === null) return null
  if (caches.has(theme)) return caches.get(theme) || null
+  if (__DEV__ && cacheValidation) {
+    const stringifiedTheme = JSON.stringify(theme)
+    if (cacheValidation.has(stringifiedTheme) {
+      console.error('Your theme is not being cached properly!')
+    } else {
+      cacheValidation.set(stringifiedTheme, true)
+    }
+  }
  const cache = {}
  caches.set(theme, cache)
  return cache
}

[mleralec]

@jguddas Thanks for your answer.

I'm not a big fan of using variables like __DEV__ in a library. This assumes you are using webpack but it won't work with vite, esbuild, rollup, swc...

I made a PR with his changes here https://github.com/gregberge/xstyled/pull/381, feel free to review it!

[theo-mesnil]

Hi @gregberge ! You can close this issue, xstyled 3.7.0 fixe it :)

Thanks @mleralec ❤️