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

[examples] MUI is missing style when navigating back from error page in remix example #30436

Closed
2 tasks done
unlinking opened this issue Dec 29, 2021 · 15 comments · Fixed by #30592
Closed
2 tasks done
Assignees
Labels
examples Relating to /examples

Comments

@unlinking
Copy link

unlinking commented Dec 29, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

MUI is missing style when back from error page in remix.

image
image
image

Expected behavior 🤔

Right style show

Steps to reproduce 🕹

Steps:

  1. Visit https://n5o0j-3000.sse.codesandbox.io
  2. Click 404 page button
  3. Click back button in browser

sandbox: https://codesandbox.io/s/awesome-voice-n5o0j

Context 🔦

It seems that styleData not reset after back from error page, this action happens only in browser not having a request to server to rerender html.

Your environment 🌎

System:
OS: Windows 10 10.0.22000
Binaries:
Node: 16.13.1 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.15 - C:\Program Files\nodejs\yarn.CMD
npm: 8.1.2 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: Not Found
Edge: Spartan (44.22000.120.0), Chromium (96.0.1054.62)
npmPackages:
@emotion/react: latest => 11.7.1
@emotion/styled: latest => 11.6.0
@mui/base: 5.0.0-alpha.62
@mui/material: latest => 5.2.6
@mui/private-theming: 5.2.3
@mui/styled-engine: 5.2.6
@mui/system: 5.2.6
@mui/types: 7.1.0
@mui/utils: 5.2.3
@types/react: latest => 17.0.38
react: latest => 17.0.2
react-dom: latest => 17.0.2
typescript: latest => 4.5.4

@unlinking unlinking added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 29, 2021
@unlinking unlinking changed the title MUI is missing style when back from error page MUI is missing style when back from error page in remix Dec 29, 2021
@mbrookes mbrookes changed the title MUI is missing style when back from error page in remix [examples] MUI is missing style when navigating back from error page in remix example Dec 29, 2021
@mbrookes mbrookes added the examples Relating to /examples label Dec 29, 2021
@mbrookes

This comment has been minimized.

@mbrookes mbrookes removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 29, 2021
@unlinking

This comment has been minimized.

@memoriesadrift
Copy link

I encountered the same bug using Next.js, so it doesn't seem to be limited to Remix.

@lvauvillier
Copy link

Same bug here. I was about to post the exact same repro.

@lvauvillier
Copy link

lvauvillier commented Dec 30, 2021

On the client, styleData is null.
If the Document component is rerendered, all the styles are lost.

Here is a workaround: pass data from server to client using a __STYLE_DATA_ global variable:

let styleData = useContext(StylesContext);

if (!styleData && typeof window !== "undefined") {
  styleData = (window as any)?.__STYLE_DATA__ as StyleContextData[] | null;
}

And

{styleData && (
   <script dangerouslySetInnerHTML={{ __html: `window.__STYLE_DATA__ = ${JSON.stringify(styleData)}` }} />
)}

This is a link to a working sample using this technique:
https://codesandbox.io/s/optimistic-cherry-dsyr5

⚠️ Edit: This technique don't work if new styles are added later navigating new pages.
See also: remix-run/remix#1136 and remix-run/remix#852

@lvauvillier
Copy link

lvauvillier commented Dec 31, 2021

I spend some time digging into this subject, and here is my understanding and how I finally solved it.
@mnajdova I'll be delighted to have your feedback on this solution

Context

Emotion, dynamically injects <style> tags in the page on demand. With Remix, error pages contain a new full html document. And when it renders it creates a mess:

  • All tags injected previously are lost
  • Emotion cache is now linked to a removed <head> tag: new tags cannot be injected
  • Global styles have also refs linked to removed elements, we need to update all refs

Solution

Re-inject tags and re-link to current <head> tag:

const Document = withEmotionCache(({ children, title }: DocumentProps, emotionCache) => {
  // Only executed on client
  React.useEffect(() => {
    // re-link sheet container
    emotionCache.sheet.container = document.head
    // re-inject tags
    const tags = emotionCache.sheet.tags;
    emotionCache.sheet.flush();
    tags.forEach(tag => {
      (emotionCache.sheet as any)._insertTag(tag);
    });
  }, []);

  return (
    <html lang="en">
      ...
    </html>
  );
});

Global styles

Global styles uses a ref to keep track on they markup. The only way to make it working is to trigger a rehydration.
Since cache is used as dependency of the rehydration side effect function, we can reset it to make it work.

Wrap client CacheProvider to add a reset function:

// ./src/ClientStyleContext.tsx

import { createContext } from 'react';

export interface ClientStyleContextData {
  reset: () => void;
}

export default createContext<ClientStyleContextData>({
  reset: () => {}
});
// entry.client.tsx

interface ClientCacheProviderProps {
  children: React.ReactNode;
}

function ClientCacheProvider({ children }: ClientCacheProviderProps) {
  const [cache, setCache] = useState(createEmotionCache());

  function reset() {
    setCache(createEmotionCache());
  }

  return (
    <ClientStyleContext.Provider
      value={{ reset }}
    >
      <CacheProvider value={cache}>
        {children}
      </CacheProvider>
    </ClientStyleContext.Provider>
  ); 
}

hydrate(
  <ClientCacheProvider>
    <ThemeProvider theme={theme}>
      {/* CssBaseline kickstart an elegant, consistent, and simple baseline to build upon. */}
      <CssBaseline />
      <RemixBrowser />
    </ThemeProvider>
  </ClientCacheProvider>,
  document,
);
// root.tsx
const Document = withEmotionCache(({ children, title }: DocumentProps, emotionCache) => {
  const clientStyleData = useContext(ClientStyleContext);

  // Only executed on client
  React.useEffect(() => {
    [...]
    // reset cache to reapply global styles
    clientStyleData.reset();
  }, []);

  [...]
}

I renamed StylesContext to ServerStyleContext to be consistant with ClientStyleContext

Updated sample:
https://codesandbox.io/s/ecstatic-shockley-jnuby

@jansedlon
Copy link
Contributor

@lvauvillier It actually works 🤯 Good job!

@unlinking
Copy link
Author

@lvauvillier It helps a lot, thanks!

@mbrookes
Copy link
Member

mbrookes commented Jan 5, 2022

Reopening for inclusion in the Remix example.

@mbrookes mbrookes reopened this Jan 5, 2022
@mnajdova
Copy link
Member

@lvauvillier I was on vacation, I've just seen your comment. Let me test it and verify that it works, and I will update the Remix example.

@mnajdova
Copy link
Member

mnajdova commented Jan 12, 2022

@lvauvillier I've created #30592 based on your codesandbox. The only problem I could notice is the flickering when going from 404 to a valid page. I think it's coming from the GlobalStyles.

Note that I've updated in the meantime the example project to not require double rendering (see remix-run/remix#1325, seems like the example don't he remix docs will be updated too).

I will look into the flickering next, it's important for us to fix it before we release this.


BTW I could notice the flickering on your sandbox too, so I don't think it's related to the changes with the double rendering.

@mnajdova
Copy link
Member

@lvauvillier never mind, I fixed it by replacing the useEffect in the Document with useLayoutEffect. I can't spot any issues anymore. Well done!

@lvauvillier
Copy link

@mnajdova yeah! I also iterate around useLayoutEffect but it is not recommended to use in component rendered server side. We get an explicit warning.

@mnajdova
Copy link
Member

@lvauvillier right, we can use the useEnhancedEffect (unstable_useEnhancedEffect) from @mui/material :)

@nojitsi
Copy link

nojitsi commented Mar 23, 2023

It's been not so easy to figure out, but solution and example helped a lot. I think now my app works even better then before:)

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

Successfully merging a pull request may close this issue.

7 participants