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.

Sign up for GitHub

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

Jump to bottom

server side rendering with multiple pages. #28

Open
Raynos opened this issue Jul 22, 2014 · 12 comments
Open

server side rendering with multiple pages. #28

Raynos opened this issue Jul 22, 2014 · 12 comments

Comments

@Raynos
Copy link
Collaborator

Raynos commented Jul 22, 2014

Let's imagine I'm building an app with rcss and want to do server side rendering.

I have two simple pages / and /about that have their own elements and styles.

Because rcss has a global registry the call to .getStylesString() would return the styles for both sites when I only want to insert the styles for a single page into that page.

How do you imagine this approach scaling ? A component cannot have a notion of "what pages am I on". The top level page component somehow needs to say "for all" styles in my render call, insert them.

Not really sure how one would do that.
@chenglou
Copy link
Owner

How about this? Your button.js exports a POJO (no rcss dependency), and the registration is done in the file requiring it (I guess for this part, we'd go back to the previous API). Redundant registration is ignored, naturally.

One of the reason I avoided this is because I'd need to hash your POJO (probably JSON.stringify) as a key for the registry. I thought I'd circumvent paying this cost with the new API/internal logic. Guess I trapped myself in a corner.

@Raynos
Copy link
Collaborator Author

Raynos commented Jul 23, 2014

@chenglou the problem is if i have a module using rcss i dont want to know about rcss when server side rendering nor do i want to know that i need to require that style and add it to the head.

so we need some kind of server side rendering solution where it can somehow render itself efficiently without duplicating things.

@Raynos
Copy link
Collaborator Author

Raynos commented Jul 23, 2014

I think I can just duplicate the virtual node for the style for an rcss object in my tree.

Then add an optimizing post processing step to remove duplicates at the top level.

@charliermarsh
Copy link
Collaborator

What if we had a separate API call, getStylesStringForMarkup(markup)? I'm envisioning something like:

  • RCSS collects the set of registered class names in the registry (without clearing it out).
  • RCSS collects the set of class names using in markup, which is some HTML string.
  • RCSS returns the CSS for the class names that appear in both sets and removes from the registry any RCSS object that's represented in the returned CSS. (This way, even if you call this function multiple times on the server for a single HTML page, any class that's used in any markup will be injected into the <head> exactly once.)

This will admittedly only work with semi-static markup (because classes that aren't represented in the initial markup wouldn't make it into the <head>). However, the initial markup would render correctly (which is what we want) and then we could act on @Raynos's suggestion in #34 (i.e., on the client-side, continue to add styles that aren't present in the <head> as-needed).

(@Raynos: I'm being sort of React-centric here because I know React makes it very easy to attain the HTML markup as a string, but from your superfine repo, it seems like it'd be easy to do with Mercury as well?)

@Raynos
Copy link
Collaborator Author

Raynos commented Aug 14, 2014

One of the approaches i was thinking was to have the rcss registry never clear itself.

It's just a lookup table from class hash to style object.

The way I would do the server side rendering is to walk my virtual tree and to ask RCSS for whats the style object for this hash.

I.e. all I need is RCSS.getStyles(hash) and I can accumulate them and put them in the <head> myself.

The biggest problem for me is the clearing of the registry.

@charliermarsh
Copy link
Collaborator

Yeah, that's an interesting idea. Two thoughts:

  1. Would it be sufficient to just have RCSS.getStyleString(hash), rather than return the actual style object? Might be nice if the API just deals with taking in hashes and returning CSS.
  2. I still think it's helpful and reasonable to clear the registry when you're using RCSS client-side. So maybe we clear the registry on injectAll, but not on getStylesString?

As a starting point (that we can tear down), I've pushed an (untested) revised API here.

@Raynos
Copy link
Collaborator Author

Raynos commented Aug 14, 2014

@crm416 I don't think you should clear the registry on the client.

You should build two registries, one of all the RCSS objects registered and one of all the class names injected into the DOM already.

Then when you call injectAll() you just see whats not injected yet and inject them.

This way there are no race conditions.

@chenglou
Copy link
Owner

(Sorry, a bit busy this week, I'll be checking the convo and issues this weekend. Also, feel free to alter the API as I don't like the current one too much).

@Raynos
Copy link
Collaborator Author

Raynos commented Aug 14, 2014

@chenglou @crm416 if either of you are in bay area this weekend i dont mind doing a rcss hack day this weekend.

@charliermarsh
Copy link
Collaborator

@Raynos Yeah, that's a much better idea. Unfortunately, I left the Bay Area last weekend and won't be back in time for a hack day. Would definitely be interested in contributing to whatever changes we decide on, though. (CC: @chenglou)

@bitplanets
Copy link

I think that if you would use proper instances instead of a global object would fix this problem. Of course it would be more difficult to use (you need to keep track of your instance trough the app).

So you might enable a global RCSS which works like how is working now.

 RCSS.registerClass({})

Then you can allow people to instance RCSS:

var myRCSS = new RCSS();
myRCSS.registerClass({})

But then you have to keep your myRCSS instance in order to render.

Another idea is to use context. For example:

 RCSS.registerClass('site1', {})

would register CSS on site1.

The problem can rise when some dependency of the app will use RCSS and you find unexpected CSS injected into your html... But that is another story.

@mattferrin mattferrin mentioned this issue Dec 28, 2014
Open
@mattferrin
Copy link

In issue #47 I comment that I like @bitplanets suggestion to allow page context more than using an object instance. I share his thought that separating pages in the code itself would simplify the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Raynos @charliermarsh @chenglou @mattferrin @bitplanets