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

Improve documentation server side rendering #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joepio
Copy link

@joepio joepio commented Dec 8, 2021

Hi there, I just started using Stitches in a new NextJS project, and I love it! Really nice stuff. The docs were clean and concise, the API is a blast to use and I really like that it supports SSR.

However, when I tried to render my stuff on the server, I saw this annoying flash, presumably because the styles were only being created when the JS was parsed. I read the docs again, but didn't notice my issue. I used dangerouslySetInnerHTML={{ __html: getCssText() }}, but no luck. When I read the output of getCssText(), I saw only my theme variables - not my components. I knew I was close to a solution. Finally, I tried changing my import from '@stitches/react' to import from '../stitches.config', and everything was fine. I felt stupid as I noticed that the docs do indeed mention that I should import this.

However, I think this message can be conveyed more clearly.

The first problem is here, in the very first 'use it' block. It shows importing styled from @stitches instead of stitches.config. I suggest changing the order, and never importing from stitches.

The second problem, perhaps more importantly, is that the Server side rendering docs don't mention this important thing.

Hi there, I just started using Stitches in a [new NextJS project](https://github.com/ontola/home), and I love it! Really nice stuff. The docs were clean and concise, the API is a blast to use and I really like that it supports SSR.

However, when I tried to render my stuff on the server, I saw this annoying flash, presumably because the styles were only being created when the JS was parsed. I read the docs again, but didn't notice my issue. I used `dangerouslySetInnerHTML={{ __html: getCssText() }}`, but no luck. When I read the output of `getCssText()`, I saw only my theme variables - not my components. I knew I was close to a solution. Finally, I tried changing my `import from '@stitches/react'` to `import from '../stitches.config'`, and everything was fine. I felt stupid as I noticed that the docs do indeed mention that I should import this.

However, I think this message can be conveyed more clearly. 

The first problem is [here](https://stitches.dev/docs/installation#use-it), in the very first 'use it' block. It shows importing `styled` from `@stitches` instead of `stitches.config`. I suggest changing the order, and never importing from `stitches`.

The second problem, perhaps more importantly, is that the [Server side rendering docs](https://stitches.dev/docs/server-side-rendering) don't mention this important thing.
@vercel
Copy link

vercel bot commented Dec 8, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/modulz/stitches-site/Ex4QkbHhV51bqNgUYJQZfFKGGRum
✅ Preview: https://stitches-site-git-fork-joepio-patch-1.modulz-deploys.com

@peduarte
Copy link
Contributor

Thanks for that, makes total sense!

@peduarte peduarte added the documentation Improvements or additions to documentation label Dec 10, 2021
@joepio
Copy link
Author

joepio commented Dec 10, 2021

Note that this PR only fixes number 2 of my suggestions. The first one is a bit more nuanced, as I can imagine you want a really easy example for non-SSR usecases.

@peduarte
Copy link
Contributor

No worries, I'll review shortly

@@ -5,6 +5,8 @@ description: How to configure server-side rendering.

You can get access to the CSS string by using the `getCssText` function. This function is made available by the `createStitches` function.

_IMPORTANT: When making components, make sure to import `styled` and other functions from `../stitches.config` instead of from `@stitches/react` inside your components. `getCssText` will only output CSS that has been created in this manner._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A suggestion to keep a consistent UI and tone of voice:

Suggested change
_IMPORTANT: When making components, make sure to import `styled` and other functions from `../stitches.config` instead of from `@stitches/react` inside your components. `getCssText` will only output CSS that has been created in this manner._
> Note: if you've [configured](/docs/installation#import-and-use-it) Stitches, ensure to import `getCssText` from your config file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this fully covers the problem. I think the user is required to do this, it would suprise me if SSR would work if importing from @stitches/react without using config, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you've not configured Stitches at all, it should work fine by importing directly from npm. Did you run into an issue where it didn't?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I didn't know that, but no, I haven't tried without setting up config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, yeah, so I think this clarifies it further. It's not perfect I agree, but we'll be making some bigger overall docs improvements!

Copy link
Contributor

@peduarte peduarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a suggestion about the copy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants