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

Remove Did not expect server HTML to contain a <style> in <div> from react-dom #16089

Closed
ImanMh opened this issue Jul 9, 2019 · 3 comments
Closed

Comments

@ImanMh
Copy link

ImanMh commented Jul 9, 2019

Do you want to request a feature or report a bug?
I'm requesting to remove a feature

What is the current behavior?
React dom shows warnings when there are style tags in between html responses from server

Warning: Did not expect server HTML to contain a <style> in <div>.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
it's not a bug but here is how to reproduce it:

Clone a SSR supported boiler plate like react-starter-kit and add this code in any component's render method:

{!process.env.BROWSER &&
    <style>{'.test { color: red; }'}</style>
}

What is the expected behavior?
not throwing warnings

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React: ^16.5.2
React-dom: ^16.5.2
Browser: FireFox 67.0.4
I think it's always been like this!

I think this warning message should be totally removed from React DOM, How should I stream using ReactDOMServer.renderToNodeStream if I don't put the style tags in between HTML tags?
I've seen many people using packages like isomorphic-style-loader which puts all the styles in the head. However if you want to steam the server response, the idea of styles in head is not possible unless you first renderToString the entire component tree which eliminates the whole purpose of streaming the results.

My suggestion is that ReactDOM does not considers style tags inside html nodes an invalid response from server so it's possible to stream server response easier and remove all these inline styles in client and move them to the head(for the sake of easier DOM manipulations in feature).

@ImanMh ImanMh changed the title Hold on a second, I think this warning message **should be totally removed from React DOM**, How do you expect to stream using ReactDOMServer.renderToNodeStream if I don't put the style tags in between HTML tags? I've seen many people using packages like [isomorphic-style-loader](https://github.com/kriasoft/isomorphic-style-loader) which puts all the styles in the head. How ever if you want to steam the response the idea of styles in the head is not possible unless you first renderToString the entire component tree which eliminates the whole purpose of streaming the results. My suggestion is that ReactDOM does not considers style tags inside html nodes an invalid response from server. Remove Did not expect server HTML to contain a <style> in <div> from react-dom Jul 9, 2019
@sebmarkbage
Copy link
Collaborator

The current streaming support isn't very good because you also can't control how content appears. It will just appear in whatever order the bytes happen to come in. It also doesn't deal with things like error handling.

We have plans for a new streaming renderer. We plan on adding support directly for this in the new renderer to be able to inject things like data and style injections as first class.

In the existing renderer, DOM that doesn't line up is a whole can of worm. We need it to line up perfectly so that we know what the surrounding elements are supposed to do. Without it we'd need too much meta data. We'll likely also want to stop even looking at the tagName for perf reasons (e.g. see #15998). So we couldn't even special case style tags.

Code like !process.env.BROWSER is in general not supported and can cause problems in many cases.

Is there a reason you can't just render the style tag on the client?

@ImanMh
Copy link
Author

ImanMh commented Jul 9, 2019

@sebmarkbage What I'm trying to do is to have SSR, CSS modules and React Render to Stream all together. I can render the style tags on the client but then I have to make sure to remove those style tags when components are removed from the DOM. The problem is that when a component has appeared in multiple places in DOM, I want to keep one single copy of it's styles (for the sake of network and CPU performance). It's easy to do in server cause I add each style only once, but in client users interact with components and they might remove them by taking an action. So keeping track of these style tags is kind of tricky in client. The simpler approach would be to move all styles into the head and keep them in one place after hydration (like isomorphic-style-loader).
During the past couple of days I've created a stream friendly version of isomorphic-style-loader which will put styles in body for server but when HTML is restored in client it will move all styles to the head. That works perfect with streaming and everything except React complains about finding style tags in body.
!process.env.BROWSER will be needed in some special cases but the main reason I wrote that was to reproduce the bug.
I would also be happy to know more about the new render and new stream features of React cause I've been doing SSR for past 3 years and it has been both disappointing and lovely at the same time. Is there any place or way to know more about it?
And also since it's not 100% for sure React will drop "looking at the tagName for perf reasons (#15998)" do you think it would be helpful to remove this warning from React to let me continue this path with current RenderToNodeStream implementation of React even thought it's not perfect at the moment?

@threepointone
Copy link
Contributor

which will put styles in body for server but when HTML is restored in client it will move all styles to the head. That works perfect with streaming and everything except React complains about finding style tags in body.

To be clear, you run some code that moves the styles to the <head>, and then on hydrating you still get the warning? That doesn't sound right, since there won't be any style tags at that point. Are you trying to hydrate before removing the style tags?

You could also consider patching console.error for this specific warning (and patching only before generating the string, and undoing the patch right after), but I don't know what your product requirements are, or whether it'll be efficient for you.

We're working on some newer ideas for server rendering, and will share them in the future. I'll close this issue for now, thank you for your question!

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

No branches or pull requests

3 participants