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

Chrome extension breaks styled in SSR #2936

Open
flappyBug opened this issue Nov 7, 2022 · 5 comments
Open

Chrome extension breaks styled in SSR #2936

flappyBug opened this issue Nov 7, 2022 · 5 comments

Comments

@flappyBug
Copy link

flappyBug commented Nov 7, 2022

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:

  • react version: ^18.2.0
  • @emotion/react version: ^11.10.5
@lucascurti
Copy link

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

@Andarist
Copy link
Member

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
Copy link
Author

flappyBug commented Nov 24, 2022

@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
Copy link

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants