Option to pass an existing element to the DOM Renderer #231

nathanmarks · 2016-05-06T15:26:33Z

Are you open to a PR that allows us to provide an element for the DOM Renderer to use instead of creating its own?

This would help me implement reconciliation with server side rendering output using the built-in renderer via jss.createStyleSheet().

The text was updated successfully, but these errors were encountered:

kof · 2016-05-06T15:40:40Z

Can you explain the goal please?

nathanmarks · 2016-05-06T15:44:09Z

@kof

I'm rendering <style> tags on the server in the document <head> so that there is no flash of unstyled content before the app is loaded.

Right now, when the client app loads up, jss is creating new stylesheets in addition to those rendered by the server.

On the client, I want to check if that stylesheet already exists in the DOM and recycle the element.

kof · 2016-05-06T15:59:36Z

I'm rendering <style> tags on the server in the document <head> so that there is no flash of unstyled content before the app is loaded.

jss is also rendering style tags in the head. what is your server side environment, no DOM?

Right now, when the client app loads up, jss is creating new stylesheets in addition to those rendered by the server.

You mean for the same components?

On the client, I want to check if that stylesheet already exists in the DOM and recycle the element.

Please explain "recycle" ... just the style element or you want to recycle the rules from CSS?

kof · 2016-05-06T16:01:52Z

What you do right now, is kinda half solved out of the box, we need to cover this anyways properly for jss users.

kof · 2016-05-06T16:02:23Z

See #221

kof · 2016-05-06T16:03:56Z

I think you are trying the second approach described in #221

nathanmarks · 2016-05-06T16:04:00Z

@kof

Converting jss styles to regular css server-side, once js components, regenerate CSS from js styles. (possible now)

That's what I'm trying to do.

I will be using the contents of the style tag until the next change. I wasn't planning on trying to do any sort of hydration just yet.

kof · 2016-05-06T16:05:05Z

Have you thought about trying the first approach?

kof · 2016-05-06T16:06:29Z

But in any case I would like to support the second approach too. So in other words you need to give jss renderer the style element which is already created on the server to use it for regeneration to avoid head pollution, right?

nathanmarks · 2016-05-06T16:07:15Z

I/we need stylesheet features which is why I am trying jss.

Right now I'm working on a proof of concept for a JS style setup to fit my use case using jss.

nathanmarks · 2016-05-06T16:07:21Z

Yep, that's exactly what we need to do.

kof · 2016-05-06T16:08:06Z

I/we need stylesheet features which is why I am trying jss.

wdym by "stylesheet features" ...

nathanmarks · 2016-05-06T16:08:59Z

@kof CSS features that you can't use inline such as pseudo classes.

kof · 2016-05-06T16:09:45Z

ok, so it was just a self contained statement, unrelated to the discussion.

nathanmarks · 2016-05-06T16:10:06Z

@kof You suggested the (1) approach which was inlining the styles

kof · 2016-05-06T16:10:42Z

no, you misunderstood the meaning of inlining there. I will expand on that in the original issue.

nathanmarks · 2016-05-06T16:12:27Z

Ah, sorry!

Anyways, back to the goal. Right now this is what happens because there is nothing marrying them:

Being able to give jss the style element via createStyleSheet() would give me the ability to use the one rendered by the server.

kof · 2016-05-06T16:14:00Z

I see 2 additional ways to avoid head pollution by style tags:

remove original style tag when the new one will be generated
render only one style tag for all styles rendered serverside

kof · 2016-05-06T16:16:03Z

You could render all styles on the server in one style element by using https://github.com/jsstyles/jss/blob/master/docs/js-api.md#a-style-sheets-registry

nathanmarks · 2016-05-06T16:27:23Z

@kof

I prefer to keep the render output on the server the same as what the client would render -- 1 stylesheet per component.

(1) is a possible alternative, but using the same node that jss rendered styles in on the server with toString() is the more natural approach to me.

kof · 2016-05-06T16:30:30Z

So how would you do it?

kof · 2016-05-06T16:34:46Z

An easy way is to pass an option to .createStyleSheet(...) and use that option within the renderer. But I already see that it's gonna be ugly when using it.

nathanmarks · 2016-05-06T16:36:15Z

Using an id/hash created from the original styles (before they are given to jss) as the meta option in createStyleSheet(). On the client, before I create a jss stylesheet I can check before creating the style sheet if a style element with the correct hash exists.

nathanmarks · 2016-05-06T16:37:54Z

Why would it be ugly? (added an example property to my current code)

styleSheet.sheet = this.jss.createStyleSheet(
  styleSheet.getRuleDefinitions(this.theme),
  { element: myElement, meta: styleSheet.name, named: false }
);

kof · 2016-05-06T16:39:44Z

Lots of questions in this code snippet, can you show a full class example?

nathanmarks · 2016-05-06T16:41:26Z

import { create as createJss } from 'jss';
import nested from 'jss-nested';
import camelCase from 'jss-camel-case';
import defaultUnit from 'jss-default-unit';

export function createStyleManager({
  jss = createJss().use(nested(), camelCase(), defaultUnit()),
  theme = {}
} = {}) {
  return Object.create(styleManager, {
    sheetMap: { value: new WeakMap() },
    jss: { value: jss },
    theme: { value: theme }
  });
}

const styleManager = {
  /**
   * Attaches a styleSheet object
   */
  attach(styleSheet) {
    const counter = this.sheetMap.get(styleSheet) || 0;

    if (counter === 0) {
      if (!styleSheet.hasOwnProperty('sheet') || styleSheet.sheet.options.jss !== this.jss) {
        styleSheet.sheet = this.jss.createStyleSheet(
          styleSheet.getRuleDefinitions(this.theme),
          { meta: styleSheet.name, named: false }
        );
      }
      styleSheet.sheet.attach();
    }

    this.sheetMap.set(styleSheet, counter + 1);
  },

  /**
   * Detach a styleSheet object
   *
   * @param  {Object} styleSheet - styleSheet Object
   */
  detach(styleSheet) {
    const counter = this.sheetMap.get(styleSheet) - 1;
    if (counter) {
      this.sheetMap.set(styleSheet, counter);
    } else {
      styleSheet.sheet.detach();
      this.sheetMap.delete(styleSheet);
    }
  }
};

nathanmarks · 2016-05-06T16:41:58Z

Note -- this file is from an experimental styling setup, I took some cues from your ideas in react-jss for it.

kof · 2016-05-06T16:46:37Z

Looks overcomplicated.

kof · 2016-05-06T16:50:58Z

Having some sort of id for a style sheet, which is predictable and unique at the same time, would make it easy to automate.

kof · 2016-05-06T16:51:24Z

maybe some hashing algorithm.

nathanmarks · 2016-05-06T16:53:44Z

@kof That's what I was going to do, but was waiting to see if I could get an option added to provide the element first.

kof · 2016-05-06T16:54:05Z

To avoid doing right now a good solution and just support your case, I can add element option to the style sheet.

kof · 2016-05-06T16:54:35Z

However I see a need for something much nicer.

nathanmarks · 2016-05-06T17:02:23Z

That would be great.

I agree that something better is needed. Right now I'm trying to validate a proof of concept to solve some issues with basic inline styling that we have been having.

I would like to discuss a solution to implement better support for this natively in jss. Once I have some conclusions I'll probably engage in some of the issues you have open such as #221

kof · 2016-05-06T17:16:25Z

added and published

kof added a commit that referenced this issue May 6, 2016

option element #231

1c427bf

kof closed this as completed May 6, 2016

Option to pass an existing element to the DOM Renderer #231

Option to pass an existing element to the DOM Renderer #231

Comments

nathanmarks commented May 6, 2016 • edited Loading

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016 • edited Loading

kof commented May 6, 2016

kof commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016 • edited Loading

kof commented May 6, 2016 • edited Loading

kof commented May 6, 2016

nathanmarks commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

nathanmarks commented May 6, 2016 • edited Loading

kof commented May 6, 2016

nathanmarks commented May 6, 2016

nathanmarks commented May 6, 2016 • edited Loading

kof commented May 6, 2016

kof commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016 • edited Loading

kof commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016

kof commented May 6, 2016

nathanmarks commented May 6, 2016 •

edited

Loading

kof commented May 6, 2016 •

edited

Loading

nathanmarks commented May 6, 2016 •

edited

Loading

kof commented May 6, 2016 •

edited

Loading

nathanmarks commented May 6, 2016 •

edited

Loading

nathanmarks commented May 6, 2016 •

edited

Loading

nathanmarks commented May 6, 2016 •

edited

Loading