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

Warning: useLayoutEffect does nothing on the server, because its effect cannot […] #15798

Closed
TidyIQ opened this issue May 23, 2019 · 53 comments
Assignees
Labels
bug 🐛 Something doesn't work external dependency Blocked by external dependency, we can’t do anything about it

Comments

@TidyIQ
Copy link
Contributor

TidyIQ commented May 23, 2019

Version 4 of <ButtonBase> now incorporates useLayoutEffect() in the src code here. This wasn't present in version 3.

I understand the need for it however every jest test for a component that has a <Button> within it now results in console warnings like so:

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://fb.me/react-uselayouteffect-ssr for common fixes.
          in ForwardRef(ButtonBase)
          in WithStyles(ForwardRef(ButtonBase))
          in ForwardRef(Button)
          in WithStyles(ForwardRef(Button)) (at ButtonWithIcon/index.tsx:57)
          in Layout (at ButtonWithIcon/index.tsx:26)
          in ButtonWithIcon (at OAuthButton/index.tsx:58)
          in Layout (at OAuthButton/index.tsx:21)
          in OAuthButton (at OAuthSignUp.tsx:221)
          in div (at OAuthSignUp.tsx:200)
          in Layout (at OAuthSignUp.tsx:138)
          in OAuthSignUp (at OAuthSignUp.unit.test.tsx:60)
          in Context.Provider
          in ThemeProvider (at testProviders.tsx:29)
          in Context.Provider (at store.tsx:36)
          in Provider (at testProviders.tsx:28)
          in Providers (at OAuthSignUp.unit.test.tsx:59)

The tests still pass as it's just a warning, but is there any way to prevent these warning messages from appearing? I literally can't even see all the warnings in my terminal as there are so many.

@eps1lon
Copy link
Member

eps1lon commented May 23, 2019

This means your test setup has a DOM while you're using sever-side rendering. I don't think ReactDOM.renderToStaticMarkup has use cases in the browser. I believe you can tell jest to run tests in a specific environment. For your SSR tests you should run in node not jsdom.


Edit by @oliviertassinari

Would it work, in your case with? (from react):

// React currently throws a warning when using useLayoutEffect on the server.
// To get around it, we can conditionally useEffect on the server (no-op) and
// useLayoutEffect in the browser.
const canUseDOM: boolean = !!(
  typeof window !== 'undefined' &&
  typeof window.document !== 'undefined' &&
  typeof window.document.createElement !== 'undefined'
);

const useIsomorphicLayoutEffect = canUseDOM ? useLayoutEffect : useEffect;

@eps1lon eps1lon added component: ButtonBase The React component. test labels May 23, 2019
@ralvs
Copy link
Contributor

ralvs commented May 25, 2019

I'm using Next.js and I'm getting the same alert but in Chrome Console when my pages loads.
Even though I have updated to the example (https://github.com/mui-org/material-ui/tree/master/examples/nextjs)

@eps1lon eps1lon self-assigned this May 25, 2019
@eps1lon
Copy link
Member

eps1lon commented May 25, 2019

I'm using Next.js and I'm getting the same alert but in Chrome Console when my pages loads.
Even though I have updated to the example (/examples/nextjs@master )

Creating a fresh copy of our example does not log any warnings for me. If you have some custom setup please include a complete reproducible example. The same applies to @TidyIQ. The issue depends on your specific environment. I would guess that you get the same warnings using the latest version of react-redux.

@TidyIQ
Copy link
Contributor Author

TidyIQ commented May 26, 2019

My code is quite large so it will take a bit of time to provide a reproduction, but just to confirm I'm also use Next.js and my tests are being run in jsdom (with jest and react-testing-library), puppeteer (with jsdom-screenshot) and ReactDOM.renderToStaticMarkup (with jest-axe). I haven't yet determined which are causing the issue yet.

@eps1lon
Copy link
Member

eps1lon commented May 26, 2019

jsdom (with jest and react-testing-library), puppeteer (with jsdom-screenshot) and ReactDOM.renderToStaticMarkup

That is the issue. You're creating an environment that is quite different from your production environment. On the server you won't have a DOM available.

For tests using ReactDOM.renderToStaticMarkup you should use @jest-environment node

@ralvs
Copy link
Contributor

ralvs commented May 27, 2019

I'm using Next.js and I'm getting the same alert but in Chrome Console when my pages loads.
Even though I have updated to the example (/examples/nextjs@master )

Creating a fresh copy of our example does not log any warnings for me. If you have some custom setup please include a complete reproducible example. The same applies to @TidyIQ. The issue depends on your specific environment. I would guess that you get the same warnings using the latest version of react-redux.

@eps1lon I found the source of issue in my case.
Its the use of next/link instead of @material-ui/core/Link

import Link from "next/link"

<Link href="/about">
  <Fab>
    <AddIcon />
  </Fab>
</Link>

<Link href='/contact' color='inherit' className={classes.link}>
    <ListItem button>
      <ListItemIcon>
        <ContactsIcon />
      </ListItemIcon>
      <ListItemText primary='Contatos' />
    </ListItem>
 </Link>

When I click in any of this links, the new page is loaded and the warnings are shown at DevTools

Just changing the import to @material-ui/core/Link resolves the issue.

@ralvs
Copy link
Contributor

ralvs commented May 30, 2019

FINALLY, I could reproduce the error! 😅
And apparently, the problem is cause when you attach next-with-apollo to the _app.js

If I use the example for Next and Apollo (https://github.com/zeit/next.js/tree/master/examples/with-apollo) the problem do not occur but I lost some good functionalities tha next-with-apollo offers

Steps to do it:

Clone the example of MUI with NextJS (https://github.com/mui-org/material-ui/tree/master/examples/nextjs)

curl https://codeload.github.com/mui-org/material-ui/tar.gz/master | tar -xz --strip=2  material-ui-master/examples/nextjs
cd nextjs
npm i
npm i next-with-apollo apollo-boost graphql

Create the HOC that create an Apollo Client (next-with-apollo.js)

import withApollo from 'next-with-apollo';
import ApolloClient from 'apollo-boost';

export default withApollo(
  ({ headers }) =>
    new ApolloClient({
      uri: 'https://localhost:4000',
      request: operation => {
        operation.setContext({
          fetchOptions: {
            credentials: 'include',
          },
          headers,
        });
      },
    })
);

Attach the HOC on _app.js

import withApollo from '../src/next-with-apollo';
...
export default withApollo(MyApp);

Add some Button to any page!
When you click a link to go to that page that have the Button, 11 warnings appear at DevTools

index.js:1 Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://fb.me/react-uselayouteffect-ssr for common fixes.
    in NoSsr
    in button
    in ForwardRef(ButtonBase)
    in WithStyles(ForwardRef(ButtonBase))
    in ForwardRef(Button)
    in WithStyles(ForwardRef(Button)) (at pages/index.js:29)
    in div
    in Styled(MuiBox) (at pages/index.js:25)
    in div
    in ForwardRef(Container)
    in WithStyles(ForwardRef(Container)) (at pages/index.js:24)
    in Index (at _app.js:29)
    in Context.Provider
    in ThemeProvider (at _app.js:26)
    in Container (at _app.js:22)
    in MyApp
    in RenderPromisesProvider
console.<computed> @ index.js:1
warningWithoutStack @ react-dom-server.browser.development.js:104
warning @ react-dom-server.browser.development.js:290
useLayoutEffect @ react-dom-server.browser.development.js:1215
useLayoutEffect @ react.development.js:1482
NoSsr @ NoSsr.js:28
processChild @ react-dom-server.browser.development.js:2887
resolve @ react-dom-server.browser.development.js:2811
render @ react-dom-server.browser.development.js:3201
read @ react-dom-server.browser.development.js:3160
renderToStaticMarkup @ react-dom-server.browser.development.js:3660
process @ react-apollo.esm.js:1038
Promise.then (async)
getMarkupFromTree @ react-apollo.esm.js:1043
getDataFromTree @ react-apollo.esm.js:1009
(anonymous) @ withApollo.js:128
step @ withApollo.js:56
(anonymous) @ withApollo.js:37
fulfilled @ withApollo.js:28
Promise.then (async)
step @ withApollo.js:30
(anonymous) @ withApollo.js:31
push../node_modules/next-with-apollo/lib/withApollo.js.__awaiter @ withApollo.js:27
_a.getInitialProps @ withApollo.js:102
_callee$ @ utils.js:33
tryCatch @ runtime.js:62
invoke @ runtime.js:288
prototype.<computed> @ runtime.js:114
asyncGeneratorStep @ asyncToGenerator.js:5
_next @ asyncToGenerator.js:27
(anonymous) @ asyncToGenerator.js:34
F @ _export.js:36
(anonymous) @ asyncToGenerator.js:23
loadGetInitialProps @ utils.js:33
_callee2$ @ router.js:373
tryCatch @ runtime.js:62
invoke @ runtime.js:288
prototype.<computed> @ runtime.js:114
asyncGeneratorStep @ asyncToGenerator.js:5
_next @ asyncToGenerator.js:27
(anonymous) @ asyncToGenerator.js:34
F @ _export.js:36
(anonymous) @ asyncToGenerator.js:23
getInitialProps @ router.js:383
(anonymous) @ router.js:245
F @ _export.js:36
(anonymous) @ router.js:243
Promise.then (async)
getRouteInfo @ router.js:235
(anonymous) @ router.js:187
F @ _export.js:36
change @ router.js:149
push @ router.js:143
SingletonRouter.<computed> @ router.js:71
Link._this.linkClicked @ link.js:125
onClick @ link.js:193
callCallback @ react-dom.development.js:149
invokeGuardedCallbackDev @ react-dom.development.js:199
invokeGuardedCallback @ react-dom.development.js:256
invokeGuardedCallbackAndCatchFirstError @ react-dom.development.js:270
executeDispatch @ react-dom.development.js:561
executeDispatchesInOrder @ react-dom.development.js:583
executeDispatchesAndRelease @ react-dom.development.js:680
executeDispatchesAndReleaseTopLevel @ react-dom.development.js:688
forEachAccumulated @ react-dom.development.js:662
runEventsInBatch @ react-dom.development.js:816
runExtractedEventsInBatch @ react-dom.development.js:824
handleTopLevel @ react-dom.development.js:4826
batchedUpdates$1 @ react-dom.development.js:20439
batchedUpdates @ react-dom.development.js:2151
dispatchEvent @ react-dom.development.js:4905
(anonymous) @ react-dom.development.js:20490
unstable_runWithPriority @ scheduler.development.js:255
interactiveUpdates$1 @ react-dom.development.js:20489
interactiveUpdates @ react-dom.development.js:2170
dispatchInteractiveEvent @ react-dom.development.js:4882

@TidyIQ
Copy link
Contributor Author

TidyIQ commented May 31, 2019

That must be it. I'm also using Apollo Client.

@eps1lon
Copy link
Member

eps1lon commented May 31, 2019

Thanks for the repro. Will check if this still happens using reduxjs/react-redux#1283.

eps1lon added a commit to eps1lon/mui-buttonbase-uselayout-repro that referenced this issue May 31, 2019
@eps1lon
Copy link
Member

eps1lon commented May 31, 2019

@ralvs Repro doesn't start. Could you create a cloneable repository or checkout https://github.com/eps1lon/mui-buttonbase-uselayout-repro and see what's missing? Followed your steps (replaced npm with yarn though). Had to install react-apollo (cannot find module) after that I get [ error ] TypeError: Cannot set property default of #<Object> which has only a getter when running yarn dev

@pedela
Copy link

pedela commented May 31, 2019

@eps1lon If I'm not mistaken, you ran into the following issue: apollographql/apollo-client#4843

Current workaround seems to be reverting to v0.3.1 or impor the client from apollo-boost/lib/index

Thanks for working on this:-)

@eps1lon
Copy link
Member

eps1lon commented May 31, 2019

@pedela Thanks for the tip. Can make it work:

$ git clone https://github.com/eps1lon/mui-buttonbase-uselayout-repro.git
$ cd mui-buttonbase-uselayout-repro
$ yarn
$ yarn dev
# goto localhost:3000/about

but I can't reproduce any error. Neither browser nor terminal console log any warnings related to useLayoutEffect.

@eps1lon eps1lon added status: waiting for author Issue with insufficient information and removed waiting for user information labels May 31, 2019
@pedela
Copy link

pedela commented May 31, 2019

Strange, I followed your steps exactly and I get:
image
Edit: Only browser console warnings though, terminal doesn't show any warnings

@ralvs
Copy link
Contributor

ralvs commented May 31, 2019

I just did the same.
Using Chrome Version 74.0.3729.169 (Official Build) (64-bit)
On MacOS Mojave v10.14.5

Screen Shot 2019-05-31 at 08 15 25

@ralvs
Copy link
Contributor

ralvs commented May 31, 2019

I also open a issue on next-with-apollo repo: #60

@eps1lon
Copy link
Member

eps1lon commented May 31, 2019

Can anbody reproduce this on a linux distribution? Maybe apollo is mocking some part of the DOM on the server on mac only?

@pedela
Copy link

pedela commented May 31, 2019

I can reproduce this exact error on an Ubuntu-based distribution.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 1, 2019

I can reproduce on MacOS 10.14.5.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 1, 2019

Ok found it. We can close this issue. You can reproduce the problem without Material-UI:

import React from "react";

const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect;

function MadeWithLove() {
  useEnhancedEffect(() => {});

  return (
    <span>
      {"Built with love by the "}
    </span>
  );
}

The problem is that next-with-apollo traverse the tree, by default, on the browser with the server side API:

I have used Apollo extensively for more than a year at onepixel.com. I would be cautious with it, it can be slow at scale, it was our bottleneck n°1 from a performance perspective (graphql + apollo client). I believe you don't need to traverse the React tree with the server side API on the client when switching routes. You can have loaders or a centralized getInitialProps logic instead.

@oliviertassinari oliviertassinari removed component: ButtonBase The React component. status: waiting for author Issue with insufficient information test labels Jun 1, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 5, 2019

I'm happy to have found the existence of ua-parser-js. I will update the ssr documentation of useMediaQuery with it, it can help when client hints are not available.

@ndeviant
Copy link

ndeviant commented Sep 5, 2019

I agree with you, and thanks for your help.
But what do you think about this:

I think simple checking for existence of window is not proper check for defining is this server or client. I think mui shouldn't tell user not to have "window" in global scope.

This took me a lot of time to find out that the reason for these warnings is that the material-ui does not correctly recognize is this server or client. And it is based on the presence of the window object

@oliviertassinari
Copy link
Member

But what do you think about this:

@ndeviant Do we have a better option?

@ndeviant

This comment has been minimized.

@ndeviant
Copy link

ndeviant commented Sep 5, 2019

@oliviertassinari oliviertassinari added the waiting for 👍 Waiting for upvotes label Sep 5, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 5, 2019

@ndeviant Sorry, this was an invitation to read the whole thread. @eps1lon has already suggested a great possible approach in #15798 (comment).

Would it work, in your case with? (from react):

// React currently throws a warning when using useLayoutEffect on the server.
// To get around it, we can conditionally useEffect on the server (no-op) and
// useLayoutEffect in the browser.
const canUseDOM: boolean = !!(
  typeof window !== 'undefined' &&
  typeof window.document !== 'undefined' &&
  typeof window.document.createElement !== 'undefined'
);

const useIsomorphicLayoutEffect = canUseDOM ? useLayoutEffect : useEffect;

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 5, 2019

@eps1lon Should we reopen and deploy a canUseDOM based solution (from React codebase)? We had 7 different people interacting with the issue in 3 months. It's not a lot, so far, its seems to surface underlying problems in people codebases. I'm happy either way and I would tend to not do anything yet.

Note that it's broader to the layout effect, we run a typeof window !== 'undefined' at a few other places.

@oliviertassinari oliviertassinari changed the title v4 of <ButtonBase> : useLayoutEffect causes many test warnings in jest Warning: useLayoutEffect does nothing on the server, because its effect cannot Sep 5, 2019
@oliviertassinari oliviertassinari changed the title Warning: useLayoutEffect does nothing on the server, because its effect cannot Warning: useLayoutEffect does nothing on the server, because its effect cannot […] Sep 5, 2019
@ndeviant
Copy link

ndeviant commented Sep 5, 2019

@oliviertassinari yeah, totally. Thats would be great!

@oliviertassinari

This comment has been minimized.

@ndeviant
Copy link

I described only one case when window on server could be useful. Initial issue creator has 9 "Thumbs up" on this, it seems like people keep running into this issue over and over again, with different circumstances (tests env, not ideal deps, etc.). My actual issue is, I've got the project, written before me, which uses window on server, and go and refactor everything that uses this logic, just to get rid of annoying warning I cannot afford.

To conclude what I was saying previously:

  1. Should 'react-urgent' working with window.navigator.userAgent, instead of context? No, for sure.
  2. Should user have 'window' object defined on server? Probably not.
  3. Is this a 'material-ui' responsibility, to say whether he could have defined 'window' on server? No. Leave it to linters or node itself, especially that warning in console ain't saying anything helpful.

React doesn't takes responsibility to say, that you shouldn't have defined 'window' on server, and checks properly for dom existence.
So you think that this isn't a bug, but a feature?

@eps1lon
Copy link
Member

eps1lon commented Sep 27, 2019

So you think that this isn't a bug, but a feature?

It's something we can't control. Unfortunately we only have useLayoutEffect for something that needs to be run during commit. Now react warns if it detects this effect when using the ssr API because it assumes that anything in the effect does affect layout. In our case it does not.

Now we solve this issue by assuming that the SSR api is used on the server and the CSR api is used on the client. But nothing prevents you from using the SSR api on the client as well (or rather in environments with a DOM) which results in these false positives.

We voiced our concern about this warning to the react core team but didn't get any response. At this point we cannot do anything more.

As far as I know there's no way of knowing if a component is rendered with the SSR api or CSR api. If there is then we could solve this issue.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 27, 2019

To the developers coming to this page. If you are affected by this problem, please upvote the issue and provide a reproduction we can look at. The more feedback, the best solution we can find. Thanks.

@KVNLS
Copy link

KVNLS commented Oct 2, 2019

Hello, I reproduced using Enzyme's render function.
Here is a sandbox

@ccdarvin
Copy link

ccdarvin commented Oct 2, 2019

I have same problema,

index.js:1 Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://fb.me/react-uselayouteffect-ssr for common fixes.
in NoSsr
in button
in ForwardRef(ButtonBase)

Code.
export default props => { return <AppBar position='sticky'> <Toolbar> <IconButton color='inherit' onClick={props.navBarToggle}> <MenuIcon /> </IconButton> </Toolbar> </AppBar> }

package:
"dependencies": { "@apollo/react-hooks": "^3.1.1", "@material-ui/core": "^4.4.3", "@material-ui/icons": "^4.4.3", "apollo-boost": "^0.4.4", "graphql": "^14.5.8", "next": "^9.0.7", "next-with-apollo": "^4.3.0", "react": "^16.10.1", "react-dom": "^16.10.1" }

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 2, 2019

@saltyshiomix
Copy link
Contributor

saltyshiomix commented Nov 1, 2019

@oliviertassinari

Still this error occurs with material-ui@4.5.2.

Internally material-ui uses layoutEffect()?

Or use "useIsomorphicLayoutEffect"?

#15798 (comment)

I want to SSR with Material UI.

If you have time, please check out the example which demonstrate this warnings:

https://github.com/saltyshiomix/react-ssr/tree/master/examples/with-jsx-material-ui

Dear,

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 1, 2019

@saltyshiomix It seems that the warning comes from https://github.com/saltyshiomix/react-ssr/blob/50895251cb950cfa4ccccf03bff43011b4303e51/packages/core/lib/webpack/material-ui.js#L7.

The usage of the server-side API on the client-side triggers these warnings.
@hburrows also reported in #17850 that the CSS is empty is in this case.

So, I would recommend not to two render twice on the client, for performance consideration. You will dodge the issue at the same time. Would that work for you?

@mui mui deleted a comment from saltyshiomix Nov 1, 2019
@mui mui deleted a comment from ndeviant Nov 1, 2019
@saltyshiomix
Copy link
Contributor

@oliviertassinari

You are perfectly right, and solved the problem!

I misunderstood the APIs, sorry.

@lcswillems
Copy link
Contributor

lcswillems commented Nov 7, 2019

@eps1lon Should we reopen and deploy a canUseDOM based solution (from React codebase)? We had 7 different people interacting with the issue in 3 months. It's not a lot, so far, its seems to surface underlying problems in people codebases. I'm happy either way and I would tend to not do anything yet.

Note that it's broader to the layout effect, we run a typeof window !== 'undefined' at a few other places.

I have exactly the same issue. In fact, everybody using material-ui and next-with-apollo with the always option will have this issue.

Using next-with-apollo with always options enable to fetch data directly from the component and this is reaaaaally convenient (it allows me to avoid to have 10 differents HOC to pass data, so much less overhead).

So if you could implement the "canUseDom" API it would be so great!!

Moreover, the issue had been closed here and opened in the next-with-apollo package, but the maintainer of this package can do nothing because the issue is coming from Material UI.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 7, 2019

@lcswillems I would recommend getDataFromTree: "never". I have used Apollo in production a year ago. getDataFromTree was a performance killer for us, we were talking of ~200 ms for this step along. In the end, we were using the equivalent of the 'ssr' option for the least frequent pages (faster dev, slower page) and 'never' (slower dev, faster page) for the most frequented pages.

I don't think that the proposal in #15798 (comment) will help your case. It's designed to people that have the issue when they render on the server and have a window that leaks, somewhere.

In your case, I believe that it's when you render on the client. As far as I know, there is nothing we can do about it. We are constrained by React.

@eps1lon
Copy link
Member

eps1lon commented Nov 8, 2019

So if you could implement the "canUseDom" API it would be so great!!

Please see #15798 (comment) for why this is not possible for libraries. Or

The problem generally is that this warning depends on what API you use not what environment you're in. So anything relying on environment sniffing will never be complete since you can use most of react-dom/server in the browser or in a node test environment with a global jsdom.

-- facebook/react#14927 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work external dependency Blocked by external dependency, we can’t do anything about it
Projects
None yet
Development

No branches or pull requests