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

Material-UI 4 example does not work properly with nextjs getStaticProps revalidate and causes wrong order of head tags #25307

Closed
2 tasks done
janus-reith opened this issue Mar 11, 2021 · 6 comments
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v4.x

Comments

@janus-reith
Copy link

  • The issue is present in the latest release (Latest stable).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When using the revalidate property of getStaticProps with nextjs. The first render of the page in production builds would be correct, but subsequent renders of the page will be incorrect, although in many cases this won't be visually notable.

When expecting the tags in the DevTools's , MuiCssBaseline would be first, but for subsequent re-renders of that page be appended to the bottom instead. (In the codesandbox example, MuiContainer would be first then)
This however is not limited to MuiCssBaseline I made some line by line comparisons on larger codebases and could notice that a lot of things where off. (So I assume forcing MuiCssBaseline probably is not the soltution)

This whole behavior happens right in the server render, and can also be observed with js in the browser disabled.

Expected Behavior 🤔

Both the first and subsequent renders of the static page should produce the same result(as long as static props dont change) with MuiCssBaseline on the top.

Steps to Reproduce 🕹

I created a quick codesandbox example here: https://codesandbox.io/s/material-ui-revalidate-bug-0joh8?file=/pages/%5Bslug%5D.js
It will create a production build on server startup to showcase the issue properly.

Steps:

  1. Enter some random path, like /test123
  2. Observe that a page is generated on demand.
  3. Inspect the HTML, check that MuiCssBaseline is on top
  4. Wait 10 seconds, refresh the page in the meantime and you'll see the still correct order
  5. Refresh the page at least twice after the 10 seconds(once to trigger a rebuild while receiving the stale page and then one time to receive the actual new page).
  6. Observe that the new created page would have a different order, with MuiCssBaseline at the bottom.

Context 🔦

While this doesn't produce any visible errors in the sandbox above, this can lead to quite some unexpected behavior on production sites.

Chances are that neither nextjs nor material-ui cause a bug here, but maybe that revalidate is feature that would make it necessary to use material-ui in a different way than originally showcased in the example.

Your Environment 🌎

`npx @material-ui/envinfo`
  Used browser: Chrome (Shouldnt be browser specific though)

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
  Binaries:
    Node: 10.23.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.8 - /usr/local/bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: Not Found
  npmPackages:
    @material-ui/core: latest => 4.11.3
    @material-ui/styles:  4.11.3
    @material-ui/system:  4.11.3
    @material-ui/types:  5.1.0
    @material-ui/utils:  4.11.2
    @types/react:  17.0.3
    react: latest => 17.0.1
    react-dom: latest => 17.0.1
@janus-reith janus-reith added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 11, 2021
@janus-reith
Copy link
Author

janus-reith commented Mar 16, 2021

@oliviertassinari Do you have any idea what could be causing this, or maybe have hints where I could start debugging this?

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 18, 2021

@janus-reith No idea. Did you try v5?

@oliviertassinari oliviertassinari added package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 18, 2021
@janus-reith
Copy link
Author

@janus-reith No idea. Did you try v5?

Yep, I couldn't reproduce it there.
Hm, I was scared by the alpha tag, but if a v4 fix is unlikely that might be the way to go.

@oliviertassinari
Copy link
Member

I would honestly imagine an issue inside Next.js. The injection order depends on the JavaScript import resolution order. With v5, you get emotion by default as a style engine that resolves the injection order differently (with props cascading). If you swap the engine to styled-components, you will likely experience the same issue as sc behaves like JSS.

@janus-reith
Copy link
Author

I would honestly imagine an issue inside Next.js. The injection order depends on the JavaScript import resolution order. With v5, you get emotion by default as a style engine that resolves the injection order differently (with props cascading). If you swap the engine to styled-components, you will likely experience the same issue as sc behaves like JSS.

Thanks, that might be a valuable hint for me, will see if I can reproduce it with SC.
I was also considering some issue revolving around next/head maybe.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 9, 2021

An update, we have now made enough progress with the new @material-ui/styled-engine package in v5 to move toward a progressive removal of the @material-ui/styles package (based on JSS). The current plan:

  • In v5.0.beta.0, this package will come standalone, in complete isolation from the rest.
  • In v5.0.0, this package will be soft deprecated, not promoted in the docs, nor very actively supported.
  • During v5, work on making the migration easier to the new style API (sx prop + styled() API). We might for instance, invest in the documentation for using react-jss that has more or less the same API. We could also invest in an adapter to restore the previous API but with emotion, not JSS.
  • In v6.0.0, this package will be discontinued (withStyles/makeStyles API).

This was made possible by the awesome work of @mnajdova.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v4.x
Projects
None yet
Development

No branches or pull requests

2 participants