emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.43k stars 1.11k forks source link

Chrome extension breaks styled in SSR #2936

Open flappyBug opened 1 year ago

flappyBug commented 1 year ago

Current behavior:

Styled is broken by some browser extensions. For example: Urban VPN Proxy

To reproduce: A minimal reproduction demo is created here with detailed description.

Expected behavior:

Extension should not affect styled if not intensional.

Environment information:

lucascurti commented 1 year ago

@flappyBug Having the same issue with Urban VPN extension in a Remix app. Did you find a workaround for it?

Andarist commented 1 year ago

It seems that Remix wants to control the whole HTML, you render it yourself here: https://github.com/flappyBug/styled-ssr-flash/blob/e46d47cb0fc9a7da1e6f9c61a92ef87a7b265813/app/root.tsx#L19-L30

and hydrate this into document itself here: https://github.com/flappyBug/styled-ssr-flash/blob/e46d47cb0fc9a7da1e6f9c61a92ef87a7b265813/app/entry.client.tsx#L7-L8

This is a little bit unusual - I've never seen React APIs used like this. Usually, you create a specific root element within your <body/> and render just that. This leaves the rest of the page outside of React's control.

Even though you have reported a problem with Emotion and the lost styling the problem is much bigger. By injecting just anything pre-hydration into a Remix app you might cause a full remount of the app, and the SSRed content gets wiped. Like, even this doesn't hold true:

interceptedSSRedBody === document.body // false

I think that there should be some guidance around this from the Remix team's side because it seems that their apps are super prone to this problem and that this can happen super easily with just any web extension.

flappyBug commented 1 year ago

@lucascurti Currently we have a workaround to remove all the elements we don't recognize before hydration. e.g. in your entry.client.tsx, before hydration:

document
  .querySelectorAll('html>:not(head,body)')
  .forEach((e) => document.documentElement.removeChild(e));
hydrate(<RemixBrowser />, document);

One drawback of this approach is that it may partially break some extensions' functionality. I didn't test it though, one may restore it after hydration:

const tamperedNodes = document
  .querySelectorAll('html>:not(head,body)');
tamperedNodes.forEach((e) => document.documentElement.removeChild(e));

hydrate(<RemixBrowser />, document);

tamperedNodes.forEach((e) => document.documentElement.appendChild(e));
lucascurti commented 1 year ago

Thanks @Andarist for looking into it and @flappyBug for the workaround. Another thing that gets rid of the issue is taking the Chakra UI approach to set up emotion: https://chakra-ui.com/getting-started/remix-guide

Andarist commented 1 year ago

hydrate doesn't have to be a synchronous operation, so I wouldn't recommend relying on this. However, most likely - it should be able to descend into body before it yields to the main loop. So... if you re-insert this synchronously after it already hydrates head but before it hydrates the whole thing then you are probably fine.

Either way - this is not a generic solution and it seems that a lot of public extensions have the potential of breaking Remix apps out there. So I'm really curious what would be the Remix team's take on this issue.