[Tabs] pseudo class ":first-child" is potentially unsafe

[mtr1990]

The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type"

an example case is: MuiTab: https://codesandbox.io/s/uzg95?file=/demo.tsx

[oliviertassinari]

@mtr1990 Thanks for the reproduction. We didn't notice the issue in the documentation because we use compat = true, which disables this warning cc @mnajdova. However, developers might decide to not use the same configuration as the one we encourage. Related threads: #24107 (comment), emotion-js/emotion#1178.

I don't think that we should encourage developers to inline <style> inside the body. It's not server-side, low-end mobile devices friendly, and it's discouraged by the HTML spec. A no go 🚫.

One fix could be to do:

diff --git a/packages/material-ui-styled-engine/src/StyledEngineProvider/StyledEngineProvider.js b/packages/material-ui-styled-engine/src/StyledEngineProvider/StyledEngineProvider.js
index 8217655f07..530bb5242b 100644
--- a/packages/material-ui-styled-engine/src/StyledEngineProvider/StyledEngineProvider.js
+++ b/packages/material-ui-styled-engine/src/StyledEngineProvider/StyledEngineProvider.js
@@ -5,6 +5,7 @@ import createCache from '@emotion/cache';

 // Cache with option to prepend emotion's style tag
 export const cache = createCache({ key: 'css', prepend: true });
+cache.compat = true;

 export default function StyledEngineProvider(props) {
   const { injectFirst, children } = props;
diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js
index 11a700c67b..f2cd9ad22e 100644
--- a/packages/material-ui/src/Button/Button.js
+++ b/packages/material-ui/src/Button/Button.js
@@ -57,17 +57,17 @@ const useUtilityClasses = (styleProps) => {

 const commonIconStyles = (styleProps) => ({
   ...(styleProps.size === 'small' && {
-    '& > *:nth-of-type(1)': {
+    '& > *:first-child': {
       fontSize: 18,
     },
   }),
   ...(styleProps.size === 'medium' && {
-    '& > *:nth-of-type(1)': {
+    '& > *:first-child': {
       fontSize: 20,
     },
   }),
   ...(styleProps.size === 'large' && {
-    '& > *:nth-of-type(1)': {
+    '& > *:first-child': {
       fontSize: 22,
     },
   }),

But then, the components would come with the warning by default. So, developers would need to provide a custom cache systematically. We can't do that.

Another possible workaround is:

      [`& .${tabClasses.wrapper} > *:first-of-type`]: {
        marginBottom: 6,
      },
      [`& .${tabClasses.wrapper} > * + *`]: {
        marginBottom: 0,
      },

But developers will have a hard time customizing it, so a go no too. The least-worse option I can think of is:

diff --git a/packages/material-ui/src/Tab/Tab.js b/packages/material-ui/src/Tab/Tab.js
index 7abf5edccc..22c2207b26 100644
--- a/packages/material-ui/src/Tab/Tab.js
+++ b/packages/material-ui/src/Tab/Tab.js
@@ -69,8 +69,8 @@ const TabRoot = experimentalStyled(
     styleProps.label && {
       minHeight: 72,
       paddingTop: 9,
-      [`& .${tabClasses.wrapper} > *:first-child`]: {
-        marginBottom: 6,
+      [`& .${tabClasses.wrapper} > * + *`]: {
+        marginTop: 6,
       },
     }),
   /* Styles applied to the root element if the parent [`Tabs`](/api/tabs/) has `textColor="inherit"`. */
@@ -145,7 +145,7 @@ const Tab = React.forwardRef(function Tab(inProps, ref) {
     icon,
     // eslint-disable-next-line react/prop-types
     indicator,
-    label,
+    label: labelProp,
     onChange,
     onClick,
     onFocus,
@@ -166,7 +166,7 @@ const Tab = React.forwardRef(function Tab(inProps, ref) {
     disableFocusRipple,
     selected,
     icon: !!icon,
-    label: !!label,
+    label: !!labelProp,
     fullWidth,
     textColor,
     wrapped,
@@ -194,6 +194,8 @@ const Tab = React.forwardRef(function Tab(inProps, ref) {
     }
   };

+  const label = typeof labelProp === 'string' && !!icon ? <span>{labelProp}</span> : labelProp;

forcing a wrapper around the label when a string for * + * to be able to take effect.

[mark-night]

Is there a way to temporarily omit/disable this error in browser console? I'm sure it'll be fixed in some future release, but seeing them keep being thrown in console is really annoying.

[oliviertassinari]

@mark-night You would need to ask the team of emotion. I was actually eager to turn off these options for the whole Material-UI project as we don't encourage to inline style in the body, but Gatsby doesn't provide the necessary API to render the style in head so I didn't push forward.

This issue includes a possible solution.

[mark-night]

@oliviertassinari I see the source of issue is from emotion or even more teams. I believe each package chooses a certain way for itself for a reason, thus this could take a long time even if it will be changed.
Meanwhile, I do see your solutions to the issue and they all seem to be simple and clear and more than enough for my need. However, they all need to modify the Material-ui source code, how do I take the advantage of your solution fix without forking the project which kicks me off future updates? Or, can I simply modify the source code in node_modules in my project? Will this cause issue for future updates? Thanks.

[oliviertassinari]

@mark-night Follow our /examples

[mark-night]

@oliviertassinari I hate to ask this stupid question, but what did you mean by examples? If you were referring to the examples folder under source root, it looks like they are just examples for how to install and setup material-ui with different tools. I'm not having any issue installing and setup material-ui in my project at all.

Your solutions were very clear and make sense, but how do I apply the solution? Do I just apply the change to my local install (via npm) in local node_modules/ ? Then wouldn't this change be overridden by next update or npm install once deployed? Kinda confused.

[oliviertassinari]

I meant that if you dig deep, you will find a solution in the /examples folder, but it's not important, what matters is that somebody champions this issue and gets a fix merged on HEAD.

[mark-night]

you will find a solution in the /examples folder

Confused, isn't the solution right here above, e.g. to modify Tab.js in the source code? My question was how to make the changes being made survive from future update (if it's still necessary for the update).

After some googling, was able to find this patch-package, good enough for now.

Thanks.

[oliviertassinari]

@mark-night The solution I had in mind is related to the compat mode of emotion. But patch-package sounds good as well. If you want to contribute a fix, we would be happy to review a PR.

[mark-night]

I finally see what you meant. :) I thought cache is only for ssr and I'm not familiar with emotion neither, didn't think down that path at all. I'll check out what emotion compact really does later.

As I'm in progress of making a pure front-end ui, your other solution of modifying Tab.js is simple and good enough, thus I went with that one.

If you want to contribute a fix

I was just copying your code. :)

[iamdevelopergirl]

Can I work on the above comments and create a PR?

[kkaplita]

Can I work on the above comments and create a PR?

I would like to see it when it is ready :)

[hbjORbj]

@iamdevelopergirl Do you have updates on this?

[jonoman55]

Does anyone know when this will be fixed? I have @mui/material": "^5.0.0" installed and I am receiving this error while using icons in Tabs. Thanks!

[iamdevelopergirl]

Hi, Apologies, I did not get time to work on it. Can someone else have a look?

[ignacio-dev]

It's 2024 and this issue still happens when you use an icon in a button. Will this be fixed?