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

manually call sheet.seal() during SSR to avoid memory leaks #329

Closed
brunorzn opened this issue Sep 28, 2018 · 8 comments
Closed

manually call sheet.seal() during SSR to avoid memory leaks #329

brunorzn opened this issue Sep 28, 2018 · 8 comments

Comments

@brunorzn
Copy link

brunorzn commented Sep 28, 2018

This is linked to styled-components/styled-components#1624 (comment)

To avoid memory leaks, we need to manually call sheep.seal during SSR, if any error has been thrown during rendering.

Here is an example :

const sheet = new ServerStyleSheet();
    try {
      const page = await renderPage((App) => (props) => sheet.collectStyles(<App {...props} />));
      const styleTags = sheet.getStyleElement();
      return {
        assets, data, ...page, styleTags,
      };
    } finally {
      sheet.seal();
    }
  }
@quantizor
Copy link
Contributor

We can probably do this in core and just rethrow the error instead of having people do this themself. What do you think @mxstbr?

@mxstbr
Copy link
Member

mxstbr commented Sep 28, 2018

Oh yeah not a bad idea!

@kitten
Copy link
Member

kitten commented Sep 29, 2018

@probablyup @mxstbr I actually think we don't need to clone on develop with SSR anymore as all global styling is now gone if I'm not mistaken 🤔

That'd get rid of the issue more elegantly 😍

@quantizor
Copy link
Contributor

oh???

@mxstbr
Copy link
Member

mxstbr commented Sep 29, 2018

@kitten I think you're right, omg. Wanna dig into that?

@wmertens
Copy link

🤔 Hmm, what version is this? S-C 4.1.1 doesn't have .complete on sheet

BTW, @brunorzn do you also use Colemak? Sheep instead of sheet :)

@quantizor
Copy link
Contributor

@wmertens wmertens changed the title manually call sheet.complete() during SSR to avoid memory leaks manually call sheet.seal() during SSR to avoid memory leaks Nov 25, 2018
@wmertens
Copy link

I updated the example and subject. @kitten not cloning at all would be better of course - afaik there is no way to add global styles any more

timneutkens pushed a commit to vercel/next.js that referenced this issue Jan 24, 2019
…nents example (#6107)

I was noticing some bad memory leak on my company's application and I ended up finding this github issue ( styled-components/styled-components#1624 ) .

This comment ( styled-components/styled-components#1624 (comment) ) caught my attention, which lead to this other issue on the repository of styled components website ( styled-components/styled-components-website#329 )

After applying the changes on my project I noticed a huge improvement on memory consumption.

So would be nice to update the example or start a discussion on how to solve this properly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants