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

Same styles are applied twice when Server-Side Rendering #22

Closed
nkzawa opened this issue Dec 7, 2016 · 9 comments
Closed

Same styles are applied twice when Server-Side Rendering #22

nkzawa opened this issue Dec 7, 2016 · 9 comments

Comments

@nkzawa
Copy link
Contributor

nkzawa commented Dec 7, 2016

It seems we have to skip generating style tags on client if it's already rendered on SSR.

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

Yeah that's a great point. I think I have an idea for this. Instead of adding expensive checks with the DOM, we could have an API like this:

import register from 'styled-jsx/register'
const elements = Array.from(document.querySelector('style.styledJSX'))
for (const el of elements) {
  register(el.getAttribute('data-jsx-id'), el)
}

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

We could also expose and manipulate styled-jsx/memory directly, but probably not a good idea?

@chiefjester
Copy link
Contributor

chiefjester commented Dec 11, 2016

@rauchg / @nkzawa here's an idea:

What if we just save the ids via global object:

rehydrate.js
import flush from 'styled-jsx/flush'

return function rehydrate(ids = 'jsx_style') {
    if (typeof window === 'undefined') return
    const styles = flush()
    window[ids] = Object.keys(styles)
}

Then on the the _JSXStyle injector, we just check for that global variable if its set, it means we check the id against the window.jsx_style variable.

@nkzawa
Copy link
Contributor Author

nkzawa commented Dec 11, 2016

How about to perform automatic detection of server rendered DOMs, which I think is similar to how React works ?

import React from 'react'
import { flushToString } from 'styled-jsx/server'
import App from './app'

function handler (req, res) {
  const app = React.renderToString(<App />)
  const styles = flushToString()
  const html = `<!doctype html>
    <html>
      <head>${styles}</head>
      <body>
        <div id="root">${app}</div>
      </body>
    </html>`
  res.end(html)
}

styles should looks like the following:

<style data-jsx-id="12345">p[data-jsx="12345"] { color: red }</style>
<style data-jsx-id="98765">p[data-jsx="98765"] { background-color: blue }</style>

Then we can automatically collect these DOMs.

I think this less flexibility is good because we want to force users to create each style element per jsx-id for ditching unused styles anyway.

@chiefjester
Copy link
Contributor

@nkzawa would style-jsx moving out of the data selector and use classNames instead? see #27

@nkzawa
Copy link
Contributor Author

nkzawa commented Dec 11, 2016

@thisguychris Both ways would work fine anyway.

About your idea, const styles = flush() is what we're discussing how we can make it work 😂

@chiefjester
Copy link
Contributor

chiefjester commented Dec 11, 2016

@nkzawa yeah the idea is adding / attaching all generated ids inside a global variable. We then add a check the existence of the variable inside renderOnClient function.

When you say

Then we can automatically collect these DOMs.

Does that mean we're going to have some DOM traversals to collect these ids from the generated styles? If so attaching an array of ids inside a window variable would be faster and simpler? Thoughts?

@nkzawa
Copy link
Contributor Author

nkzawa commented Dec 11, 2016

@thisguychris

Does that mean we're going to have some DOM traversals to collect these ids from the generated styles?

yep, ids and elements

If so attaching an array of ids inside a window variable would be faster and simpler?

I can't fully imagine how it works. To achieve this, we have to force users to write some JS on client ? Additionally, we need to get style elements too. just collecting ids is not enough.

@chiefjester
Copy link
Contributor

chiefjester commented Dec 11, 2016

@nkzawa if we have the ids, we can add a check inside renderOnClient function. If the id is there, then we skip rendering the <style ...?

nkzawa pushed a commit that referenced this issue Dec 19, 2016
* 'text/css' is not required (https://www.h3xed.com/web-development/is-type-text-css-needed-for-style-and-link-tags)

* add high-level server rendering APIs to avoid duplicates (#22)

* verify existing server-rendered DOM to avoid duplicate styles

* expose `styled-jsx/server`

* add server rendering tests

* yarn

* avoid checking the DOM unnecessarily after removing a style

* README style fix

* fix linting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants