stitchesjs / stitches

[Not Actively Maintained] CSS-in-JS with near-zero runtime, SSR, multi-variant support, and a best-in-class developer experience.
https://stitches.dev
MIT License
7.72k stars 252 forks source link

SSR: getCssText accumulates all styles from all pages #820

Open bacher opened 2 years ago

bacher commented 2 years ago

Bug report

Describe the bug

Function getCssText accumulates all styles what it even seen on the project.

It happens when using stitches framework on projects with SSR (with dynamic page rendering (not static mode)). Dynamic page rendering is important condition, static rendering works nice.

Problem: Client gets all styles from all pages, not from page he navigates to.

Looks like this problem will appear on any SSR frameworks, I'm trying to use Stitches with Next.js.

I think thing what I want called as Critical Path CSS extraction, it works in styled-components, emotion, etc.

To Reproduce

I've prepared repo with code to reproduce the problem: https://github.com/Bacher/next-stitches-bug-report

  1. Start project (yarn build && yarn start)
  2. Navigate to index page (http://localhost:3000/)
  3. If we look at CSS from SSR it will be okay, only styles what uses on the page
  4. Then navigate to page called "heavy styled"
  5. And return back to index page (/)
  6. Hit F5 (refresh page, for SSR process)
  7. If we look at CSS at this moment, CSS from SSR will contain all styles from index page AND all styles from heavy page.

Sooner or later styles from SSR start to include all styles from all pages from project.

Expected behavior

Expected: SSR CSS for any page will contain only css for that page (not for all pages at once).

Screenshots

Initial styles for index page: image

Styles for index page after going to heavy page and back: image

System information

Additional context

None

flex-haegul commented 2 years ago

Interesting.

jonathantneal commented 2 years ago

The getCssText function expresses the accumulated CSS, initially from the page, and then from each page thereafter. This is happening in a lifecycle on the server. As long as the server remains in the same lifecycle, Stitches doesn’t know that the page has changed.

If you would like to reset the Stitches lifecycle yourself on each page, you can use the reset() function exported from createStitches().

bacher commented 2 years ago

I think there is maybe some problem, on SSR several clients can be processed simultaneously, some requests could wait for fetching data, other one can render, some can be in sending response state. I don't know where the right place to put this hook. This requires some specific knowledge about inner structure of SSR framework. I could try, but I will not be sure is it optimal, and can't be sure what it cover all corner cases.

Ideally I expected some example of how to fix it for example in Next.js if it possible. It could help a lot of people in future.

csantos1113 commented 2 years ago

+1 on this one. I think it'll be beneficial to see a blog post or a dedicated docs page with the details of what @jonathantneal is referring and how the point raised by @bacher could be handled.

I found this blog post https://stitches.dev/blog/using-nextjs-with-stitches from September 2020, but it's pretty basic

thetutlage commented 2 years ago

I think each page needs to call createStitches to have an isolated state

MarkosKon commented 2 years ago

The blog post for gatsby on step 4 makes it seem that the getCssText function returns the critical path CSS for each page, not the accumulated CSS from all pages on the server lifecycle.

Another confusing thing is that if I call reset before getCssText, the pages have no styles, but if I call reset after getCssText, each page seems to have the correct(?) styles. Maybe change the name of reset to what we reset? I have a gatsby repo if someone wants to take a look.

Edit: For a solution see this pull request https://github.com/radix-ui/design-system/pull/360/files.

cpakken commented 2 years ago

I think it would be nice to see updated docs on how to use getCssText with nextjs. Especially when there are weird cases where it outputs all accumulated styles for every page instead of the critical path extraction for the specific page.

An idea for a possible implementation: A lazyGetCssText function that triggers getCssText() and then resets right afterward. It stores the result into the cache which maps onto a specific path. Any subsequent calls will just return the css text from the cache.

function createGetLazyCssText() {
  let cache: Record<string, string> = {}

  return Object.assign(
    (path: string) => {
      if (cache[path]) return cache[path]
      const css = (cache[path] = getCssText()) 
      reset()
      return css
    },
    {
      resetCache: () => (cache = {}),
    }
  )
}

export const lazyGetCssText = createGetLazyCssText()
stefanonava77 commented 5 months ago

any news on this topic?