vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.94k stars 26.98k forks source link

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

Closed karlhorky closed 4 years ago

karlhorky commented 4 years ago

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:

https://github.com/vercel/next.js/blob/dd264f582f547508b5b5af2cf1dc665a9c53f8e5/examples/with-emotion/pages/_app.js#L5

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 commented 4 years ago

cc @timneutkens

mattcarlotta commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

mattcarlotta commented 4 years ago

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 commented 4 years ago

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

mattcarlotta commented 4 years ago

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

oliviertassinari commented 3 years ago

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 https://github.com/mui-org/material-ui/issues/26561#issuecomment-855286153. 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 commented 2 years ago

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.