Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

CSS module styling is removed too early on route changes #17464

Closed
cmwall opened this issue Sep 30, 2020 · 107 comments
Closed

CSS module styling is removed too early on route changes #17464

cmwall opened this issue Sep 30, 2020 · 107 comments
Labels
linear: next Confirmed issue that is tracked by the Next.js team. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Webpack Related to Webpack with Next.js.

Comments

@cmwall
Copy link

cmwall commented Sep 30, 2020

Bug report

Describe the bug

CSS module styling is removed immediately after clicking a next/link, instead of after the DOM is removed on production builds. This causes the components to have no styling at all during a page transition. This issue does not happen on dev mode.

I believe this is a bug with CSS modules specifically because components styled with styled-jsx don't have this problem.

Really would love to be able to use Sass via CSS modules here instead of re-writing the entire app I'm working on using styled-jsx. If Sass modules can't work in this scenario, I think I would be forced to use styled-jsx, which is not my preferred method of styling my components for this project.

To Reproduce

I have created repos, and deployed these repos to demonstrate the problem using framer-motion for page transitions. If you were to pull these repos and run them locally using npm run dev, you will see that the flash of unstyled content does not happen on any one of them in dev mode. However, on their deployed sites, you can see the flash of unstyled content with CSS modules and Sass modules.

styled-jsx

Behavior: correct, no flash of unstyled content
Deployed site on Vercel
Repo

CSS modules

Behavior: buggy, there is a flash of unstyled content immediately after clicking the link
Deployed site on Vercel
Repo

Sass via CSS modules (additional)

Behavior: buggy, there is a flash of unstyled content immediately after clicking the link (same as CSS modules)
Deployed site on Vercel
Repo

Expected behavior

Styling for components that come from CSS modules should not be removed immediately on route changes, and instead, are removed when the markup is removed (the component unmounts?). The expected behavior is the behavior we can see on the styled-jsx deployment above.

System information

  • OS: macOS
  • Browser (if applies): N/A
  • Version of Next.js: 9.5.3
  • Version of Node.js: 12.14.1

NEXT-1351

@MihirGH
Copy link

MihirGH commented Oct 3, 2020

I looked at this issue and this seems to be happening because of the behaviour implemented in onCommit method.

Inside onCommit, we are setting the media attribute to the style tags which are not desired too early.

Exploring this further and will raise a PR for this in sometime

@MihirGH
Copy link

MihirGH commented Oct 3, 2020

Essentially what I ended up finding was: we should never set media attribute to x on the <style> tags since they might still be in use even though the page that was using it is being replaced. To do this, we simply need to get rid of the code that is executing in the else block of this loop in onCommit.

I don't know if this was done because of any specific reasons or not but this seems to solve the issue on my local after making these changes.

@lfades any thoughts? If it looks good to you then I'll go ahead and raise a PR for this

@MihirGH
Copy link

MihirGH commented Oct 3, 2020

Also, to make this work and to have the <style> tags on SSRed page, we might need to remove the isInitialRender check in onStart and !isInitialRender check in onCommit

@ccambournac
Copy link

Hi, experiencing the very same issue as reported.

@scriptify
Copy link

scriptify commented Oct 18, 2020

Experiencing the same issue. I also noticed the media attribute being set to x on the relevant style tags. Would love to avoid the switch to styled-jsx, as it doesn't play well with framer motion: framer/motion#231

I've applied this hack in the mean time:

    // Add that code to _app.tsx / _app.jsx

    import Router from "next/router";

    const routeChange = () => {
      // Temporary fix to avoid flash of unstyled content
      // during route transitions. Keep an eye on this
      // issue and remove this code when resolved:
      // https://github.com/vercel/next.js/issues/17464

      const tempFix = () => {
        const allStyleElems = document.querySelectorAll('style[media="x"]');
        allStyleElems.forEach((elem) => {
          elem.removeAttribute("media");
        });
      };
      tempFix();
    };

   Router.events.on("routeChangeComplete", routeChange );
   Router.events.on("routeChangeStart", routeChange );

I am not sure what other unwished side effects this temporary fix could have, but it seems to work just well enough for my application.

@tommhuth
Copy link

I'm having the same issue, but @scriptify's fix does not work for me, any ETA for a fix for this? @MihirGH

@MihirGH
Copy link

MihirGH commented Oct 22, 2020

@tommhuth It is actually a quick fix but I am not sure if that's how it is supposed to be solved or not.

@scriptify
Copy link

@MihirGH That code is just a temporary workaround, it also possible that Next.js is setting the media attribute slightly after those events fire, maybe that's the reason why it's not working for @tommhuth. @tommhuth are you sure that in your case the problem is also related to the media attribute?

@tommhuth
Copy link

Having looked into this, I can't see anywhere that media attribute is being changed, but I do see that the relevant link has its rel set from stylesheet to preload. If I manually revert it to stylesheet the appropriate styles are back, but when I do this in a routeChangeComplete/routeChangeComplete event callback there is a flash of unstyled content so those events are probably not accurate enough to serve as a quick fix. Also, I don't think I have any way of knowing exactly what style link is needed, so I end up setting all links with css files back to stylesheet which might be unnecessary. @scriptify @MihirGH

Off the top of my head, the only way I could get around this is by moving all style modules into non-module SCSS, but that is quite a significant workaround.

@scriptify
Copy link

Hmm that's strange, seems like a severe issue to me, and moving everything into non modular CSS seems like a very tedious task, just to make it modular again when it's fixed (besides the whole disadvantages global CSS has). Are we the only ones using CSS Modules + non-instant route changes? 🤷‍♂️ Doesn't seem too exotic to me

@Limekiller
Copy link

@scriptify I was running into this issue as well; your hack worked for me for the time being, so thanks for that.

@fredcorr
Copy link

@scriptify I bumped into the same issue. could you help me understand where I would add the fix you wrote? Thanks for your help as well

@Limekiller
Copy link

@fredcorr I put the code in _app.js along with, of course, import Router from 'next/router'

@scriptify
Copy link

@fredcorr Yea that code is a bit out of context, as @Limekiller mentioned, the best place to put it is _app.tsx / _app.jsx. I updated my comment accordingly to clear that up.

@fredcorr
Copy link

@Limekiller @scriptify thanks guys that fixed the issue partially, on the first-page transition the issue still occurs. Are you guys using the getStaticProps or GgetServerSideProps? Could that maybe affect it? @MihirGH Any updated on how long will it take to fix this?

@scriptify
Copy link

@fredcorr I'm using getStaticProps for all my pages for this particular project, but I don't think that should affect the route transition behaviour, just speculating ofc. Would be nice to get some hints from the Next.js team 🤗

@tommhuth
Copy link

tommhuth commented Nov 9, 2020

For anyone needing a dirty quick fix for this one until a fix is released, simply importing the modules whose style is needed in _app.js solved it for me without requiring any other refactoring.

@jdlk07
Copy link

jdlk07 commented Dec 2, 2022

@danieljamesross I was having the same issue with styled-components on page load or page refresh. Just needed to create a .babelrc file in your root and add the following

{
  "presets": ["next/babel"],
  "plugins": [["styled-components", { "ssr": true }]]
}

This was mentioned in the docs somewhere but I overlooked it. Not sure if it will work for you but it did for me.

@ArthurGoupil
Copy link

Coming with a non-solution to a problem that persists... If you're stuck; here is an approach, as ridiculous as it is; but keep in mind this is to be tested again when updating nextjs for obvious reasons

The function :

  const savePageStyles = () => {
    const head = document.head
    const previousStylesFixes = head.querySelectorAll('[data-fix]')

    // Delete previously created fixes
    if (previousStylesFixes) {
      for (let i = 0; i < previousStylesFixes.length; i++) {
        head.removeChild(previousStylesFixes[i])
      }
    }

    // Get all the styles of the page
    const allStyleElems = head.querySelectorAll(
      'link[rel="stylesheet"], link[as="style"]'
    )
    // Get all the inline styles of the page, labelled by "data-n-href" ( defined by nextjs )
    const allInlineStylesElems = head.querySelectorAll('style[data-n-href]')

    // Create doubling links to css sheets that wont be removed unless we say so
    if (allStyleElems) {
      for (let i = 0; i < allStyleElems.length; i++) {
        if (allStyleElems[i].href) {
          const styles = document.createElement('link')
          styles.setAttribute('data-pt-fix', 'true')
          styles.setAttribute('rel', 'stylesheet')
          styles.setAttribute('href', allStyleElems[i].href)

          head.appendChild(styles)
        }
      }
    }

    // Now do the same with the inline styles
    const inlineStyles = document.createElement('style')
    inlineStyles.setAttribute('data-pt-fix', 'true')
    if (allInlineStylesElems) {
      for (let i = 0; i < allInlineStylesElems.length; i++) {
        inlineStyles.innerHTML += allInlineStylesElems[i].innerHTML
      }

      head.appendChild(inlineStyles)
    }
  }

When I call it :

import { useRouter } from 'next/router'
...

const router = useRouter()
...

// On router change, trigger that function
useEffect(() => {
  const handleDone = () => {
    // Arbitrary setTimeout, modify it at will
    setTimeout(() => {
      savePageStyles()
    }, 1000)
  }

  router.events.on('routeChangeComplete', handleDone)

  return () => {
    router.events.off('routeChangeComplete', handleDone)
  }
}, [router])

// Trigger the onMount function on the first page load so that your first page is also "fixed"
// Note : I let you decide how this function is triggered because this depends a lot per stack
const onMount = () => {
  savePageStyles()
}

I wish this non-solution to be quickly unuseful.

This is working as a temporary fix.
To make the removal work I had to fix small typo
const previousStylesFixes = head.querySelectorAll('[data-fix]')
->
const previousStylesFixes = head.querySelectorAll('[data-pt-fix]')

@wolfgangschoeffel
Copy link

@JoshuaHolloway in _app.js...

import { usePageTransitionFix } from '@/hooks'

export default function NextApp({ Component, pageProps }) { 
 // ? Temp fix for FOUC on route change...  :(
  usePageTransitionFix()
// more app code...
}

use-page-transition-fix.js hook...

import Router from 'next/router'
import { useEffect } from 'react'

export const OPACITY_EXIT_DURATION = 1

const routeChange = () => {
  const tempFix = () => {
    const elements = document.querySelectorAll('style[media="x"]')
    elements.forEach((elem) => elem.removeAttribute('media'))
    setTimeout(() => {
      elements.forEach((elem) => elem.remove())
    }, OPACITY_EXIT_DURATION * 1000)
  }
  tempFix()
}

export const usePageTransitionFix = () => {
  console.debug(
    'WARNING: Still using FOUC temp fix on route change.  Has the Next.js bug not been fixed?  See https://github.com/vercel/next.js/issues/17464',
  )
  useEffect(() => {
    Router.events.on('routeChangeComplete', routeChange)
    Router.events.on('routeChangeStart', routeChange)

    return () => {
      Router.events.off('routeChangeComplete', routeChange)
      Router.events.off('routeChangeStart', routeChange)
    }
  }, [])

  useEffect(() => {
    const asPath = Router.router?.asPath
    Router.router?.push(asPath)
    // ? Use replace() instead of push()?
  }, [])
}

Make sure to clear the timeout when routeChange is called, otherwise when the user navigates back during the transition, the styles of the current page will be removed.

Also when using query params like this:

useEffect(() => {
    const pathname = Router.router?.pathname
    const query = Router.router?.query
    Router.router?.push({ pathname, query })
  }, [])

a 404 or 500 will redirect to /_error instead of displaying an error message on the current page.

This is my modification. It can probably be improved. Just wanted to point out these two issues because I already saw them in the wild.

import Router from 'next/router'
import { useEffect } from 'react'

export const OPACITY_EXIT_DURATION = 1

export const useTransitionFix = () => {
  useEffect(() => {
    let timeout
    const routeChange = () => {
      clearTimeout(timeout)
      const elements = document.querySelectorAll('style[media="x"]')
      elements.forEach((elem) => elem.removeAttribute('media'))
      timeout = setTimeout(() => {
        elements.forEach((elem) => elem.remove())
      }, OPACITY_EXIT_DURATION * 1000)
    }

    Router.events.on('routeChangeComplete', routeChange)
    Router.events.on('routeChangeStart', routeChange)

    return () => {
      Router.events.off('routeChangeComplete', routeChange)
      Router.events.off('routeChangeStart', routeChange)
      clearTimeout(timeout)
    }
  }, [])

  useEffect(() => {
    const pathname = Router.router?.pathname
    const query = Router.router?.query
    if (pathname === '/_error') {
      return
    }
    Router.router?.push({ pathname, query })
  }, [])

  return null
}

@shaianest
Copy link

had the same issue with work around above but my css module is in _app and the hack has flicker even without timeout
my next version is 12.3.4 and i'm having multiple other apps that work as expected.

ttucker pushed a commit to ttucker/tavis.me that referenced this issue Jan 9, 2023
…rably show the homepage tagline when navigating from home page to interior page.

Note to self: follow vercel/next.js#17464 for resolution and revert this change if possible.
@Kethatril
Copy link

Using this fix https://github.com/moxystudio/next-with-moxy/blob/master/www/app/use-fouc-fix.js from @satazor has fixed any issues for us (calling useFoucFix() in _app.jsx)

@KingBael09
Copy link

I've got a workaround without using the fix by @satazor!

By importing component dynamically (next/dynamic).....the problem of CSS-disabling goes away.

// index.js

import dynamic from "next/dynamic";

const Component1 = dynamic(() => import("@path/component1"));

const Component2 = dynamic(() => import("@path/component2"));

export default function Home() {
  return (
    <>
      <Component1/>
      <Component2/>
    </>
  );
}

However Page should contain only components & each component has its own CSS Module.

This workaround works in my case Next.js 13.1.2 and framer-motion 8.5.2

@lorenzomigliorero
Copy link

lorenzomigliorero commented Jan 29, 2023

As an alternative, I was looking for a way to completely avoid CSS splitting. It doesn't' make much sense to me, especially for tiny chunks. Changing the optimisation.splitchunks.minsize setting isn't enough, so apparently, there's no way to avoid CSS splitting (even for < 1kb file!).
As a very drastic but effective solution, I merged all the chunked CSS into the global stylesheet (assuming that it starts with the html selector):

// your_script.js

const path = require('path');
const fs = require('fs');

const CSS_PATH = path.resolve(__dirname, './.next/static/css');

const CSS_FILENAMES = fs.readdirSync(CSS_PATH);

const GLOBAL_CSS_FILENAME = CSS_FILENAMES.find((file) => {
  const FILE_PATH = path.join(CSS_PATH, file);
  const data = fs.readFileSync(FILE_PATH, 'utf8');
  return data.startsWith('html');
});

const GLOBAL_CSS_PATH = path.join(CSS_PATH, GLOBAL_CSS_FILENAME);

CSS_FILENAMES.filter((name) => name !== GLOBAL_CSS_FILENAME).forEach((file) => {
  const FILE_PATH = path.join(CSS_PATH, file);
  const data = fs.readFileSync(FILE_PATH, 'utf8');

  fs.appendFileSync(GLOBAL_CSS_PATH, data);
  fs.truncate(FILE_PATH, 0, () => console.log('done'));
});
{
  "scripts": {
    "build": "next build",
    "postbuild": "node your_script.js"
  }
}

@claus
Copy link

claus commented Feb 28, 2023

Oh, this thread is still alive.

I have an update on the hook i posted here almost two years ago: #17464 (comment)

The new version below solves a couple of things:

  • All dynamic page styles except the latest two are now being removed as new page styles come in (this could be improved further, but is good enough for our use cases).
  • We ran into weird behavior with the original hook where removing media="x" wasn't fast enough to prevent the browser from noticing and recalculating styles and running layout, but fast enough for it to not run paint and composition, resulting in scroll position jumping up without anything in the page visually changing. Took me a while to figure this one out.

Just call this hook in your _app and you should be all set.

[[[ THIS IS BUGGY AND OUTDATED, SEE UPDATE ]]]

import * as React from 'react';

export const useNextCssRemovalPrevention = () => {
    React.useEffect(() => {
        // Remove data-n-p attribute from all link nodes.
        // This prevents Next.js from removing server rendered stylesheets.
        document.querySelectorAll('head > link[data-n-p]').forEach(linkNode => {
            linkNode.removeAttribute('data-n-p');
        });

        const mutationHandler = (mutations: MutationRecord[]) => {
            mutations.forEach(({ target, addedNodes }: MutationRecord) => {
                if (target.nodeName === 'HEAD') {
                    // Add data-n-href-perm attribute to all style nodes with attribute data-n-href,
                    // and remove data-n-href and media attributes from those nodes.
                    // This prevents Next.js from removing or disabling dynamic stylesheets.
                    addedNodes.forEach(node => {
                        const el = node as Element;
                        if (el.nodeName === 'STYLE' && el.hasAttribute('data-n-href')) {
                            const href = el.getAttribute('data-n-href');
                            if (href) {
                                el.setAttribute('data-n-href-perm', href);
                                el.removeAttribute('data-n-href');
                                el.removeAttribute('media');
                            }
                        }
                    });

                    // Remove all stylesheets that we don't need anymore
                    // (all except the two that were most recently added).
                    const styleNodes = document.querySelectorAll('head > style[data-n-href-perm]');
                    const requiredHrefs = new Set<string>();
                    for (let i = styleNodes.length - 1; i >= 0; i--) {
                        const el = styleNodes[i];
                        if (requiredHrefs.size < 2) {
                            const href = el.getAttribute('data-n-href-perm');
                            if (href) {
                                if (requiredHrefs.has(href)) {
                                    el.parentNode!.removeChild(el);
                                } else {
                                    requiredHrefs.add(href);
                                }
                            }
                        } else {
                            el.parentNode!.removeChild(el);
                        }
                    }
                }
            });
        };

        // Observe changes to the head element and its descendents.
        const observer = new MutationObserver(mutationHandler);
        observer.observe(document.head, { childList: true, subtree: true });

        return () => {
            // Disconnect the observer when the component unmounts.
            observer.disconnect();
        };
    }, []);
};

@claus
Copy link

claus commented Feb 28, 2023

For the Next team (cc @wyattjoh)

To help mitigate this issue, maybe you could add a next.config.js flag that, if enabled, lets app developers opt out of the current behavior.

A couple ideas how this could behave:

  1. opt out of the removal of styles altogether (and perhaps a hook or API that provides a cleanup function that users can call when their page transition is done, which sets media="x" on old style elements, and does whatever else has to be done)
  2. instead of removing styles for old pages right away, keep styles for the previous page and only remove those that are older than that

The flag could be called pageCSSCleanup and take the values "auto" | "manual" | "delayed", with "auto" being the default, enabling the current behavior, "manual" enabling option 1 and "delayed" option 2.

@DangerousJack
Copy link

@danieljamesross I was having the same issue with styled-components on page load or page refresh. Just needed to create a .babelrc file in your root and add the following

{
  "presets": ["next/babel"],
  "plugins": [["styled-components", { "ssr": true }]]
}

This was mentioned in the docs somewhere but I overlooked it. Not sure if it will work for you but it did for me.

this worked for me. Just had to install babel-styled-components, create a babel config and paste in the setup to use the styled components plugin. So far so good! Thanks!

@panteliselef
Copy link

@DangerousJack I think this works only if you are using babel and not SWC, which is not the case for most people as Next.js is using SWC by default these days

@Masta2000
Copy link

Same problem here, i hope they will fix that soon (maybe in 1-2 years at this rate :3 ) but thx to @claus his solution works like a charm while waiting for a more official fix ! Thx dude

@mxydl2009
Copy link

with "next": "13.1.6" and "framer-motion": "^10.10.0", this page transition problem is still alive, and through above solutions, can solve the problem in my project. thank you!

@claus
Copy link

claus commented Apr 15, 2023

I have an update on the hook i posted here almost two years ago: #17464 (comment)

Just a heads up, that updated hook i posted above assumes that one page loads exactly one stylesheet. That's not always the case. In some configurations, pages load more than one stylesheet, breaking my hook. I'll have a fix for that soon.

@claus
Copy link

claus commented May 7, 2023

Ok, probably my last post on this issue :)

We published a PageTransition suite of components, that includes the useNextCssRemovalPrevention hook I've been posting about here (#17464 (comment) and #17464 (comment)). It prevents Next.js from removing page styles too early, but removes unused styles eventually. It addresses all problems I'm currently aware of.

npm i @madeinhaus/nextjs-page-transition

import { useNextCssRemovalPrevention } from '@madeinhaus/nextjs-page-transition'

Source
https://github.com/MadeInHaus/haus-core/blob/main/packages/nextjs-page-transition/src/helpers/useNextCssRemovalPrevention.ts

Docs
https://hauscore.xyz/components-nextjs/page-transition
https://hauscore.xyz/components-nextjs/page-transition#usenextcssremovalprevention

Demo
https://next-starter-ts.hauscore.xyz (ts)
https://next-starter.hauscore.xyz (js)

@nicholasruggeri
Copy link

@claus <3

@baptistebriel
Copy link

Thank you @claus 🙏🏻

GavrilenkoGeorgi added a commit to GavrilenkoGeorgi/jsmonkey that referenced this issue Jun 17, 2023
- Another dep for page transitions, previous one was causing page
style breakage by removing css too early
vercel/next.js#17464
@timneutkens timneutkens added linear: next Confirmed issue that is tracked by the Next.js team. and removed good first issue Easy to fix issues, good for newcomers labels Jul 3, 2023
@ArianHamdi
Copy link
Contributor

Ok, probably my last post on this issue :)

We published a PageTransition suite of components, that includes the useNextCssRemovalPrevention hook I've been posting about here (#17464 (comment) and #17464 (comment)). It prevents Next.js from removing page styles too early, but removes unused styles eventually. It addresses all problems I'm currently aware of.

npm i @madeinhaus/nextjs-page-transition

import { useNextCssRemovalPrevention } from '@madeinhaus/nextjs-page-transition'

Source https://github.com/MadeInHaus/haus-core/blob/main/packages/nextjs-page-transition/src/helpers/useNextCssRemovalPrevention.ts

Docs https://hauscore.xyz/components-nextjs/page-transition https://hauscore.xyz/components-nextjs/page-transition#usenextcssremovalprevention

Demo https://next-starter-ts.hauscore.xyz (ts) https://next-starter.hauscore.xyz (js)

Love U

@vercel vercel locked and limited conversation to collaborators Apr 17, 2024
@balazsorban44 balazsorban44 converted this issue into discussion #64658 Apr 17, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
linear: next Confirmed issue that is tracked by the Next.js team. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Webpack Related to Webpack with Next.js.
Projects
None yet
Development

No branches or pull requests