[Stack] doesn't space children correctly when there are style tags between them

cmd-johnson commented 1 year ago

Duplicates

[X] I have searched the existing issues

Latest version

[X] I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/unruffled-pine-lnym3m?file=/src/App.tsx

Steps:

Create a <Stack> with spacing > 0 (e.g. spacing={2})
Add multiple children to the Stack
Add a <style /> tag between any of those children

Current behavior 😯

The child immediately following the <style /> tag has zero margin towards the previous non-<style /> child.

Expected behavior 🤔

The child immediately following the <style /> tag has the same margin towards the previous non-<style /> child as the other elements.

Context 🔦

When using emotion's renderStylesToNodeStream for SSR, <style> tags are inserted in front of every node that is styled using emotion:

<Stack spacing={2}>
  <Box sx={{ background: 'red', width: 200, height: 50 }} />
  <Box sx={{ background: 'green', width: 200, height: 50 }} />
  <Box sx={{ background: 'blue', width: 200, height: 50 }} />
</Stack>

results in the following markup being sent to the client:

<style data-emotion="css 1p5q5e5-MuiStack-root">
  <!-- styles generated by emotion -->
</style>
<div class="css 1p5q5e5-MuiStack-root">
  <style data-emotion="css 1wt8xa8"><!-- ... --></style>
  <div class="MuiBox-root css-1wt8xa8"></div>
  <style data-emotion="css v40qv"><!-- ... --></style>
  <div class="MuiBox-root css-v40qv"></div>
  <style data-emotion="css 1007pot"><!-- ... --></style>
  <div class="MuiBox-root css-1007pot"></div>
</div>

The <style> tags get moved into the <head> tag after client hydration, which makes the boxes display correctly with spaces in between them. That means that the layout shifts after hydration, which just doesn't look right.

Your environment 🌎

npx @mui/envinfo

Tested with both Firefox and Chromium (105.0.5195.102) ``` System: OS: Linux 5.10 Manjaro Linux Binaries: Node: 18.12.0 - ~/.n/bin/node Yarn: 1.22.19 - /usr/bin/yarn npm: 8.19.2 - ~/.n/bin/npm Browsers: Chrome: Not Found Firefox: 104.0.2 npmPackages: @emotion/react: ^11.10.4 => 11.10.4 @emotion/styled: ^11.10.4 => 11.10.4 @mui/base: 5.0.0-alpha.103 @mui/codemod: 5.10.11 @mui/core-downloads-tracker: 5.10.11 @mui/docs: 5.10.9 @mui/envinfo: 2.0.6 @mui/icons-material: 5.10.9 @mui/joy: 5.0.0-alpha.51 @mui/lab: 5.0.0-alpha.105 @mui/markdown: 5.0.0 @mui/material: 5.10.11 @mui/material-next: 6.0.0-alpha.59 @mui/private-theming: 5.10.9 @mui/styled-engine: 5.10.8 @mui/styled-engine-sc: 5.10.6 @mui/styles: 5.10.10 @mui/system: 5.10.10 @mui/types: 7.2.0 @mui/utils: 5.10.9 @mui/x-data-grid: 5.17.9 @mui/x-data-grid-generator: 5.17.9 @mui/x-data-grid-premium: 5.17.9 @mui/x-data-grid-pro: 5.17.9 @mui/x-date-pickers: 5.0.6 @mui/x-date-pickers-pro: 5.0.6 @mui/x-license-pro: 5.17.0 @types/react: ^18.0.21 => 18.0.21 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 styled-components: 5.3.6 typescript: ^4.8.4 => 4.8.4 ```

siriwatknp commented 1 year ago

We don't support zero config SSR. Please take a look at our example to see the SSR configuration. https://github.com/mui/material-ui/blob/master/examples/nextjs-with-typescript/pages/_document.tsx

siriwatknp commented 1 year ago

<style> should not be added in the middle of the Stack because it does not work well with CSS. I don't think we can do anything about this.

cmd-johnson commented 1 year ago

Thank you for looking into this, @siriwatknp! While I know that <style> tags outside the <head> are not exacly standard-conform, the fact is that they works in every browser I have tested so far. It is also the approach that emotion uses for supporting streamed rendering in React.

The fix I proposed in #34966 is simple and fixes these issues in the latest Chrome, Firefox and WebKit browsers.

I have a medium sized application using the streamed rendering approach and so far it's working excellent. The only issue during hydration is this (easily changed) CSS sibling selector inside the Stack component.

oliviertassinari commented 1 year ago

The same issue was reported in #37338. I agree with @cmd-johnson, I think that we can solve this, we recommend not to inline

mui / material-ui