-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
implement gatsby-plugin-jss #1431
implement gatsby-plugin-jss #1431
Conversation
Deploy preview ready! Built with commit ac3d08f |
Deploy preview failed. Built with commit ac3d08f https://app.netlify.com/sites/gatsbygram/deploys/59778bc9424ef2018a48eaab |
This looks awesome! Thanks! Any reason not to use https://www.gatsbyjs.org/docs/browser-apis/#onClientEntry? |
Deploy preview ready! Built with commit ac3d08f |
@KyleAMathews, when |
8d4fa84
to
5d5bf77
Compare
So from this discussion it looks like they've solved the SSR/client hydration issues for the next version of JSS https://github.com/cssinjs/react-jss/pull/107 I'd rather not add an special-case API to work around a bug in another library... could we just add JSS support with the client styles hydration not quite working knowing this will get fixed soon on the next release? |
@wizardzloy Ok with taking out the new API? Would love to get this in soon! |
Also could you add a very simple example site similar to the example sites for other css plugins. |
@kof could you please confirm that with
Could you please provide a link to one of those for a reference? |
Removing critical css style tag is still required after application was mounted. Just added a note to the readme of react-jss. It was previously documented in the core only. We never perceived this as a bug and had no plans to change it for now for basically 2 reasons:
Also note that not removing it might have no negative consequences. It will though if resulting runtime css differs to the ssr and conflicts, leading to ssr overriding the client-side. This might happen though in some cases, so better remove it. |
@kof I don't understand why removing the SSR styles is necessary. Are styles generated differently during SSR? Why would it matter if they're there or not? |
JSS has http://cssinjs.org/json-api?v=v8.1.0#function-values which is mutative. On the server, one specific state is used, on the client, state might change over time, but the same CSS rule is used which has the same selector, but a different property. |
But if the initial props used are the same on both server and client we're fine? |
yes, as long as props don't change at runtime, also not in the future.
…On Jul 15, 2017 17:18, "Kyle Mathews" ***@***.***> wrote:
But if the initial props used are the same on both server and client we're
fine?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1431 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADOWJ7VVtZhfDLzNyax3uwYIJAiavp5ks5sONg5gaJpZM4OR5zG>
.
|
@KyleAMathews I really like the idea of keeping the API surface as small as possible but doesn't |
Mb it helps if you show more useful use cases. I understand the hesitation for adding a new hook function which needs to be maintained when there is only one particular use case which sounds like an optional. Mb its good to see a general usefulness using some examples. |
@@ -1,17 +1,24 @@ | |||
{ | |||
"name": "gatsby-plugin-jss", | |||
"version": "1.0.1", | |||
"description": "Stub description for gatsby-plugin-jss", | |||
"version": "2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the version to 1.0.0
Ok, so to be safe they'd definitely need removed. This is a low-risk API to add and there's perhaps other use cases and I'd definitely like to support JSS. Let's change the name of the API though to |
5d5bf77
to
8fe5882
Compare
this also adds a new lifecycle callback "onInitialClientRender" to Browser API. It is called when the initial render of a Gatsby App is done on the client.
8fe5882
to
5831e38
Compare
@KyleAMathews Thanks for the review and feedback, I changed the name of the hook to |
Awesome! Really happy to have a JSS plugin added! |
Deploy preview failed. Built with commit ac3d08f https://app.netlify.com/sites/using-glamor/deploys/59778bca424ef2018a48eabd |
Deploy preview failed. Built with commit ac3d08f https://app.netlify.com/sites/using-contentful/deploys/59778bcb424ef2018a48eac3 |
Hiya @wzrdzl! 👋 This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here. Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! 💪💜 |
this also adds a new lifecycle callback
onClientRender
to Browser API.It is called when the initial render of a Gatsby App is done on the client.