dlehmhus commented 3 years ago

Bug report

Describe the bug

If you replace (as seen in the with-stitches example)

export default class Document extends NextDocument {
  render() {
    return (
      <Html lang="en">
        <Head>
          <style
            id="stitches"
            dangerouslySetInnerHTML={{ __html: getCssText() }}
          />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    )
  }
}

with (required if you want to enable the concurrent features of React 18)

export default function Document() {
  return (
    <Html lang="en">
      <Head>
        <style
          id="stitches"
          dangerouslySetInnerHTML={{ __html: getCssText() }}
        />
      </Head>
      <body>
        <Main />
        <NextScript />
      </body>
    </Html>
  );
}

only the stitches theme gets applied to the document head, but not the styles of the rendered components. (On the server).

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

Go to CodeSandbox
Inspect the styling in the html file send from the server

Expected behavior

All styles of the current page should be added.

Screenshots

example

System information

OS: macOS 12.0.1
Version of Stitches: 1.2.5
Version of Next.js: 12.0.4
Version of react: 17.0.2
Version of react-dom: 17.0.2
Version of Node.js: 14.17.0

peduarte commented 3 years ago

I don't think Stitches is meant to work with React 18 yet, we'd need to make some changes. But Im unsure if this particular example should work.

/cc @hadihallak can you help out?

dlehmhus commented 3 years ago

@peduarte this example actually uses react 17. This is basically the example with-stitches repo. Except for the _document file of course :)

hadihallak commented 3 years ago

@dlehmhus Yeah this looks like a bug — we will be looking into it. Thanks for reporting it 🙏

akevinge commented 3 years ago

Sorry for referencing this issue so many times (am fairly noobish at programming in general and didn't realize it worked like that!).

I actually still had this issue with a class Document component and fixed it by returning the styles tag in getInitialProps like this:

export default class Document extends NextDocument {
  static async getInitialProps(ctx: DocumentContext) {
    try {
      const initialProps = await NextDocument.getInitialProps(ctx);

      return {
        ...initialProps,
        styles: (
          <>
            {initialProps.styles}
            <style
              id="stitches"
              dangerouslySetInnerHTML={{ __html: getCssText() }}
            />
          </>
        ),
      };
    } catch (e) {
      console.error(e.message);
    } finally {
      null;
    }
  }
  render() {
    return (
      <Html lang="en">
        <Head></Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    );
  }
}

System Information:

Windows 10
- Stitches: 1.2.5
- Next.js: 11.1.0
- react: 17.0.2
- react-dom: 17.0.2
- Node.js: 16.8.0

hadihallak commented 2 years ago

Hey @dlehmhus 👋 I did some digging in here and it seems that nextjs renders the page before the render method is called when using a class based _document that extends the NextDocument class and that is the correct order of events required for the getCssText to work correctly. This however does not happen when using a functional component is used as the _document so what this results in, is having stitches collect the styles before the page is even rendered so in order to have things work correctly, we’d need to mock this behavior of a class based document and we can do that by adding a getInitialProps on the function document component :

Document.getInitialProps = async function getInitialProps(ctx) {
  // render page
  const results = await ctx.defaultGetInitialProps(ctx);
  // get the css for the current rendering cycle
  const stitchesCssString = getCssText();
  // reset the styles between renders
  reset();
  return {
    ...results,
    styles: (
      <>
        {results.styles}
        <style
          id="stitches"
          dangerouslySetInnerHTML={{ __html: stitchesCssString }}
        />
      </>
    )
  };
};

you can test it here https://codesandbox.io/s/functional-document-8cqmn?file=/pages/_document.jsx

hadihallak commented 2 years ago

@innub would you mind sharing a full re-production of what you encountered as i'm not able to reproduce the issue in here https://codesandbox.io/s/class-document-p353i?file=/pages/_document.jsx

akevinge commented 2 years ago

@innub would you mind sharing a full re-production of what you encountered as i'm not able to reproduce the issue in here https://codesandbox.io/s/class-document-p353i?file=/pages/_document.jsx

I am also unable to reproduce the issue. Pretty weird because I looked through my commit history and was always using a class Document component, but I definitely had the exact same issue. Sorry for the trouble, and I'll make an issue if something comes up! Thanks for this awesome library 👍

dlehmhus commented 2 years ago

Hey @hadihallak, thanks for taking the time to look into this issue. Adding getInitalProps on the function document component does the trick. But I'm not sure if this violates the "no static getInitialProps" requirement described in the docs. But then again, as @peduarte mentioned in the beginning, stitches is not supposed to work with react 18 anyways...

CeamKrier commented 2 years ago

Hey @dlehmhus 👋 I did some digging in here and it seems that nextjs renders the page before the render method is called when using a class based _document that extends the NextDocument class and that is the correct order of events required for the getCssText to work correctly. This however does not happen when using a functional component is used as the _document so what this results in, is having stitches collect the styles before the page is even rendered so in order to have things work correctly, we’d need to mock this behavior of a class based document and we can do that by adding a getInitialProps on the function document component :
Document.getInitialProps = async function getInitialProps(ctx) {
  // render page
  const results = await ctx.defaultGetInitialProps(ctx);
  // get the css for the current rendering cycle
  const stitchesCssString = getCssText();
  // reset the styles between renders
  reset();
  return {
    ...results,
    styles: (
      <>
        {results.styles}
        <style
          id="stitches"
          dangerouslySetInnerHTML={{ __html: stitchesCssString }}
        />
      </>
    )
  };
};
you can test it here https://codesandbox.io/s/functional-document-8cqmn?file=/pages/_document.jsx

I can confirm that the issue still persists on:

"next": "12.3.1"
"react": "18.2.0"
"@stitches/react": "^1.2.8"

and the solution above does the trick

EDIT: I did not include the reset call. It was working without it.

stitchesjs / stitches

Next.js Error: Styles don't get correctly applied if _document exports a functional component #881