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

Need update in example code about external stylesheet #2413

Closed
mocheng opened this issue Jun 30, 2017 · 18 comments
Closed

Need update in example code about external stylesheet #2413

mocheng opened this issue Jun 30, 2017 · 18 comments
Labels
examples Issue was opened via the examples template.

Comments

@mocheng
Copy link

mocheng commented Jun 30, 2017

In example code about external stylesheet https://github.com/zeit/next.js/tree/master/examples/with-global-stylesheet , it still has dangerouslySetInnerHTML. According to this styled-jsx improvement` vercel/styled-jsx#148, we can reference imported stylesheet now.

I tried to change code and it works.

 export default () =>
   <div>
-    <style dangerouslySetInnerHTML={{ __html: stylesheet }} />
+    <style jsx global>{stylesheet}</style>
     <p>ciao</p>
   </div>

The dangerouslySetInnerHTML does not look right and might trigger eslint error. If my understanding is right, I'll submit a PR for this.

@timneutkens
Copy link
Member

@giuseppeg this won't work right? Since it's not a JS export

@timneutkens timneutkens added Status: Awaiting Response examples Issue was opened via the examples template. labels Jul 2, 2017
@eliot-akira
Copy link
Contributor

In my case, using <style jsx global> for the stylesheet results in this:

Module build failed: SyntaxError: The Identifier stylesheet is either undefined or it is not an external StyleSheet reference i.e. it doesn't come from an import or require statement

@giuseppeg
Copy link
Contributor

@timneutkens correct, styled-jsx transpiles only exported js strings/template literals.
Maybe with something like webpack raw-loader you can make it work but afaik only client side code gets bundled via webpack so it won't work for SSR.

@eliot-akira in your case the error could be due to the fact that you don't import the stylesheet.

@timneutkens
Copy link
Member

@giuseppeg thanks for the confirmation 👌

@mocheng
Copy link
Author

mocheng commented Jul 7, 2017

@giuseppeg @timneutkens Sorry to bother you, but it does work in SSR.

With this commit mocheng@7ac857a , the server-side-rendered HTML has .foo{color:green} in its <style id="__jsx-style-14162456061"> tag of <head>.

I understand that @giuseppeg 's point is that webpack config only affects browser side bundling. But, this change proves that it work in SSR by injecting style into inline style.

@giuseppeg
Copy link
Contributor

afaik only client side code gets bundled via webpack so it won't work for SSR
@mocheng this was more like "I think that...", I was looking for someone to confirm :)

It seems that in the example webpack is doing the magic, but if it would be very cool if that works for ssr too. Have you also tried without the global property on the style tag?

@mocheng
Copy link
Author

mocheng commented Jul 10, 2017

@giuseppeg without global, the inline style tag would have something like .foo[data-jsx-ext~="24162456061"]{color:green} which is scoped style.

With global, the inline style tag would have .foo{color:green}.

@timneutkens
Copy link
Member

This is really cool 😲

@giuseppeg
Copy link
Contributor

I will give it a try, if this works it is amazing because it means that people can write regular CSS in separate .css files and benefit from tooling etc.

@giuseppeg
Copy link
Contributor

giuseppeg commented Jul 10, 2017

Alright @mocheng it works indeed! Apparently a combo of babel-plugin-wrap-in-js and webpack's raw-loader does the trick. But I wouldn't use it in production yet because the babel plugin doesn't have tests and when doing server side rendering styled-jsx writes the css in a style tag in the page which means that it is never cached therefore if you have a lot of css and big external files your page size can grow considerably.

Since there is a solution though we might explore ways to allow this in a performant way – maybe we should write external compiled files for SSR in actual .css files and then load them using a regular link tags rather than an inline style.

@mocheng
Copy link
Author

mocheng commented Jul 11, 2017

@giuseppeg I once tried to utilize ExtractTextPugin to extract CSS/SCSS into bundle CSS files to be referenced in style tag in head, but it is hard (if not impossible) to do it by customizing webpack config. I bet it would be easier if next.js itself provides such feature.

@giuseppeg
Copy link
Contributor

I think that we could do that in styled-jsx I'll think about it and write a proposal. The api could be <style jsx as="link">

@ilionic
Copy link

ilionic commented Jul 16, 2017

Noticed styles/index.scss is bundled in commons.js
Basically same css duplicated - inlined into the page and part of app.js
Any tips how to exclude it from app.js?
image

@giuseppeg
Copy link
Contributor

giuseppeg commented Jul 17, 2017

for regular global styles files I would just serve them from /static and include them in the page with a link tag. You can use a prebuild script to copy or build them into static.

@ilionic
Copy link

ilionic commented Jul 17, 2017

So is it normal behaviour with bundling?
In my case I have external component-based .styl files that are imported and injected to <style jsx> so it's not really regular global styles.

import ComponentStyles from 'components/my-component.styl';
...
<style jsx>{ComponentStyles}</style>

@giuseppeg
Copy link
Contributor

@ilionic it'll be inlined in the page when you do server side rendering and in your bundle in case you load that page on the client (via router/ajax)

@giuseppeg
Copy link
Contributor

see also the conversation above #2413 (comment)

@ilionic
Copy link

ilionic commented Jul 18, 2017

Ok I see, it's about finding compromise between performance, page size and dev experience.
Currently using combination of both methods - global css file with some theme dependencies ( like bootstrap ) above the fold loaded using link tag in non-render blocking way. It's rarely changed and cacheable.

Critical css above-the fold is inlined ( using style jsx ) also it's nicer dev experience having component based styles with hot reload.
It works but as you pointed @giuseppeg page size started growing considerably ( including js bundle )
So until we can migrate from global css to css-in-js solutions looking for possible optimisations.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 31, 2019
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.
Projects
None yet
Development

No branches or pull requests

5 participants