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

Update examples/with-emotion to remove _document.js? #17626

Closed
karlhorky opened this issue Oct 5, 2020 · 9 comments · Fixed by #17651
Closed

Update examples/with-emotion to remove _document.js? #17626

karlhorky opened this issue Oct 5, 2020 · 9 comments · Fixed by #17651
Labels
examples Issue was opened via the examples template. good first issue Easy to fix issues, good for newcomers

Comments

@karlhorky
Copy link
Contributor

Feature request

Is your feature request related to a problem? Please describe.

The examples/with-emotion example is using Emotion 10.

According to the Emotion SSR docs, the _document.js file is no longer required:

Screen Shot 2020-10-05 at 15 04 17

Suggested Solution: Maybe this file can be removed?


Secondly, I noticed that the cache is using the original emotion as a dependency:

import { cache } from 'emotion'

Suggested Solution: Maybe this can be switched to use the createCache function from @emotion/cache instead?

Describe the solution you'd like

Solutions suggested above.

Describe alternatives you've considered

Leave things how they are (more code, but maybe still works?)

@karlhorky
Copy link
Contributor Author

cc @timneutkens

@lfades lfades added good first issue Easy to fix issues, good for newcomers examples Issue was opened via the examples template. labels Oct 5, 2020
@mattcarlotta
Copy link
Contributor

mattcarlotta commented Oct 6, 2020

Switching to @emotion/cache without extracting styles in the _document appears to be working well in development and in production. Ran some e2e tests using cypress in staging, as well as some unit tests with jest-emotion, and they all passed. In terms of performance, appears to be no loss switching between the two in production:

Before (custom document): Lighthouse
After (cache): Lighthouse

Then again, I only use @emotion/styled with the @emotion/babel-preset-css-prop babel preset, so you (or whoever) may want to test their other APIs (like jsx and css from @emotion/core, as well testing in v11.0.x-x-next) before completely removing the _document page from the official example(s).

@mattcarlotta
Copy link
Contributor

mattcarlotta commented Oct 7, 2020

Hold off on updating the official example, I'm experiencing FOUC during initial page load when not using the _document page!

For example....

without document (slow 3g):

with document (slow 3g):

@karlhorky
Copy link
Contributor Author

Thanks for the heads up! Copied your comment to #17651 .

@mattcarlotta
Copy link
Contributor

mattcarlotta commented Oct 7, 2020

Update: I was able to replicate the FOUC when using getStaticProps with revalidate:

Update: No longer an issue. See comment below.

1.) Clone repo: git clone git@github.com:mattcarlotta/emotion-fouc.git

2.) Cd into project: cd emotion-fouc

3.) Install, build and start local build: yarn && yarn build && yarn start

4.) Visit http://localhost:3000/ (should load page without FOUC), then do a hard refresh at least 5 times: ctrl/cmd + f5 (should notice FOUC).

5.) Open chrome tab, open dev tools, under Network tab change the Online option to Slow 3G and hard refresh again.

6.) The page will show FOUC during initial page load:

and styled components once loaded:

@karlhorky
Copy link
Contributor Author

Cool reproduction, thanks! Let's make sure this isn't in the final example!

@mattcarlotta
Copy link
Contributor

Just chiming in to say that following this comment, FOUC doesn't appear to be an issue anymore.

@kodiakhq kodiakhq bot closed this as completed in #17651 Oct 29, 2020
kodiakhq bot pushed a commit that referenced this issue Oct 29, 2020
@karlhorky karlhorky changed the title Update examples/with-emotion to remove _document.js? And upgrade to @emotion/cache? Update examples/with-emotion to remove _document.js? Nov 18, 2020
@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jul 28, 2021

IMHO, using emotion without extracting critical CSS in the head should no be encouraged (what this PR does). I'm listing the downsides of the approach I can find in mui/material-ui#26561 (comment). But we will see how this turns into with the new server React components and Suspense SSR support in React 18. I have no idea.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue was opened via the examples template. good first issue Easy to fix issues, good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants