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

Add with-extracted-stylesheet example #3451

Closed
wants to merge 3 commits into from

Conversation

sheerun
Copy link
Contributor

@sheerun sheerun commented Dec 13, 2017

This is something I've been working for some time, and I think it is finally Good Enough™

This example shows how to setup production-grade next.js app with extracted stylesheets:

  • Hot reloading in development without CSS-in-JS
  • 100% Page Speed score and 100% YSlow score (if put behind CDN)
  • Critical stylesheets injected per-device (phone, tablet, desktop)
  • SCSS and PostCSS with autoprefixing for styling
  • Content hashes for all resources for best integration with CDNs
  • Properly setup Cache headers for resources
  • Auto-bundling images referenced as url() in stylesheets
  • Auto-bundling of imported and required images
  • Inlining small images as Data URI (in both stylesheets and SSR page)
  • Properly handling multiple pages and routing between them
  • Ability to fallback to dynamic pages
  • Importing single icons from Font Awesome
  • GZip compression of served pages

I made injecting critical stylesheets optional because it takes a long time to install critical. There are also two methods of deploying: now --npm that skips critical css injecting, and now --docker that takes longer but injects them. I developed https://github.com/sheerun/extracted-loader to achieve hot reloading of extracted stylesheets in development.

All optimizations make first meaningful paint available in less than 200ms, even though the app-part of application is quite heavy (ACE-editor).

The demo is available for testing at:
https://with-extracted-stylesheet.now.sh/

I recommend testing for example with https://gtmetrix.com/

I hope you accept this example or even better: incorporate some of the ideas in next.js (e.g. per-device rendering, Link integrated with exported pages, stylesheet bundling baked in, hashes).

@davidqhr
Copy link

Look forward to use this feature !!!!

@schoenwaldnils
Copy link
Contributor

@sheerun I have the feeling that this example does too much at once. You have a lot dependencies that don't seem to be necessary to get the extracted stylesheets working.
Could you break it down to only the function needed to work with extracted stylesheets?

@sheerun
Copy link
Contributor Author

sheerun commented Dec 19, 2017

I disagree, every part of this example is necessary to pass performance test on 100% on each page on each device type. If you strongly feel I should remove something, please point it explicitly. I could rename it to with-performant-stylesheet or something.

@schoenwaldnils
Copy link
Contributor

In my opinion the example-section of next.js should be as simple as possible.
And that it would be obvious that you need everything in this example to get this technique going in your project.
For example this example: https://github.com/zeit/next.js/tree/canary/examples/with-external-scoped-css
The pages/index.js there has nothing I wouldn't need.

Why do I need the editor.js? Why do I need a hero or your Logo?

Also you do you need a lot of techniques to get the performance to 100%.
You can archive that with using a combination of examples and techniques.
But that shouldn't be the goal of an example.

@sheerun
Copy link
Contributor Author

sheerun commented Dec 19, 2017

editor.js is to showcase splitting that page loads with 100% performance on main page even though search.js is bloated with ace editor. Hero page showcases few important features: inlining background image into external css, inlining small logo file into html (with url-loader), shows that you can use modern SCSS framework and still achieve hot reloading and critical css, shows how to import single font-awesome icons so to avoid bloating external stylesheet.

In my opinion this is a comprehensive and minimal example how to implement production-ready external css that at the same tame showcases all features so to prevent regressions.

Indeed the example name could be with-performant-stylesheet instead with-external-stylesheet. Next already has the example how to implement non-performant external css (with-global-stylesheet), so there's no point for me to prepare striped version of this example.

@sheerun
Copy link
Contributor Author

sheerun commented Jan 10, 2018

Can this be merged?

@saxxi
Copy link

saxxi commented Jan 11, 2018

Hi,

I think is okay for the page to "heavy" in kb size so we can load test, however this has a some complexity.

While I think is okay for files like navbar.js and link.js, I feel bin/start & bin/bundle have too much code and ultimately hard to maintain.

Another issue it seems yarn start requires yarn build which is not ideal for development.

I think this PR would be great in the repository, can this be fixed?

PS I also have the error in the with-external-scoped-css issue.

@sheerun
Copy link
Contributor Author

sheerun commented Jan 13, 2018

To be honest, I don't think so. I tried to be as concise as possible in these scripts. Yarn start is not supposed to be used in development, but production. In development you use yarn dev, just like in every other next.js example. next start requires next build as well.

@saxxi
Copy link

saxxi commented Jan 17, 2018

@sheerun Sorry my bad, yarn start is for production (I'm new to next).

"name": "with-next",
"version": "1.0.0",
"scripts": {
"dev": "node bin/start",
Copy link

Choose a reason for hiding this comment

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

due to the code written (process.env.NODE_ENV === 'development' ) please use:

NODE_ENV=development node bin/start

@sheerun
Copy link
Contributor Author

sheerun commented Jan 17, 2018

I've fixed issue with env variables

]
})

config.module.rules.push({
Copy link
Member

Choose a reason for hiding this comment

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

guess this can be changed to @zeit/next-css now that it uses extracted-loader 🕵️

@timneutkens timneutkens mentioned this pull request Feb 22, 2018
@hugotox
Copy link

hugotox commented Feb 28, 2018

This example is awesome but I agree next.js examples are way smaller. This one has a dockerfile and a bin folder which are usually per project kind of files, not next.js specific examples

@sheerun sheerun closed this Apr 19, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Apr 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants