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

Server-side rendering blockers #40

Closed
rclai opened this issue Jul 4, 2015 · 20 comments
Closed

Server-side rendering blockers #40

rclai opened this issue Jul 4, 2015 · 20 comments

Comments

@rclai
Copy link
Contributor

rclai commented Jul 4, 2015

Since it looks like SSR is now doable, it looks like this issue and this issue might be closer to getting properly addressed at the core level for SEO's sake?

@stubailo
Copy link
Contributor

stubailo commented Jul 7, 2015

Thanks for bringing it up! There are a few more things we need to do to explicitly support SSR as well.

@arunoda
Copy link
Contributor

arunoda commented Jul 16, 2015

@stubailo I've solved 90% of the problems around SSR. HTTP response header is not an issue. This involve one hack which I used for fast-render. I know @glasser is working on a proper fix for that.

So, I'd like talk about this and what are things still in the queue. It's better if you could do an open discussion.

@stubailo
Copy link
Contributor

Can we do the open discussion here? Seems as good a place as any. What are the 10% of remaining problems?

@stubailo stubailo changed the title SSR: HTTP response headers Server-side rendering blockers Jul 16, 2015
@arunoda
Copy link
Contributor

arunoda commented Jul 16, 2015

Sure :)

@arunoda
Copy link
Contributor

arunoda commented Jul 17, 2015

Here's what I completed. (But needs heavy testing)

  • Create a environment for SSR. So when we running client side code, we can fetch data correctly
  • Send the rendered HTML (hacking node http.Output message)
  • Send the fast-render data
  • Add support for isomorphic titles and metatags (still a private repo. Releasing soon)
  • React server rendering is super expensive. So, implemented a caching layer
  • 404 handling is theoretically okay. Looking for an API for FlowRouter. Can be done with not-found

Here's a working version based on that. All the pages in this side load from SSR. (pages are on the db)
See: http://fresh.kadira.io/platform

If this site is down, then go to https://kadira.io

So, these are working pretty well and needs more testing.

Here's the missing 10%

Now this SSR is for basically for search engines and bots. For real users (loggedIn users), SSR won't matter and we should not do it. Because it's super expensive in CPU wise.

But for them, we can just use FastRender. Because that's something pretty efficient. But there's a catch. In order to identify subscriptions, we need to run components on the server. So, not way to get around with it.

So, now I'm thinking on a system to guess subscriptions based on routes. It needs to render components few times. Then, it'll learn subscriptions for a given route. Then, in the next time it can do fast-rendering.

Here's what we need from MDG

Here's I used some safe hacks (used for fast-render from the beginning) to send HTML. But I think MDG is working on a way to do it.

Then, we also need a way to customize the response header and status as well.

@stubailo
Copy link
Contributor

Wait, I thought you just said we don't need response headers from MDG.

So to summarize you want:

  1. A real way to send content from the server without hacks
  2. Customize response headers/code/etc

Doesn't seem like much! Is that all?

What are additional things you would need to reduce the amount of hacks?

@arunoda
Copy link
Contributor

arunoda commented Jul 17, 2015

Wait, I thought you just said we don't need response headers from MDG.

I didn't get this properly. We need to a way to tell to change response headers for the current request. Then, we can do server side redirects and other stuff. So, best idea is to add some APIs to the nodeJS http.OutgoingMessage or provide a low-level server routing library (like connect) with all these APIs.

Yeah! I told you it's not much :)

Oh. I forgot another thing. There is another hack. (not a hack a bit kind of overriding thing).
Now, Mongo collection APIs needs to know about the SSR context. Then, they need to handle differently.
So, I did it with a override like this: https://github.com/meteorhacks/flow-router/blob/ssr/server/plugins/ssr_data.js#L20

But, I think for now it's good since we are pretty not sure about how SSR Context is going to change and so on.

@arunoda
Copy link
Contributor

arunoda commented Jul 17, 2015

We can do SSR for Blaze as well. So, make templates available on the server as well. It was there earlier. But, removed it to reduce the bundle size. Bring it back :)

@AdamBrodzinski
Copy link
Contributor

One blocker I have with react-router SSR is an error that gets thrown when rendering to a string. From what I can gather this vague error is common when you forget to use module.exports so i'm thinking it's an npm / browserfy / exposify issue. I'll try to dig into this later next week.

https://github.com/AdamBrodzinski/meteor-react-ssr-spike/issues/1

TypeError: Object #<Object> has no method 'toUpperCase'
    at autoGenerateWrapperClass (packages/node_modules/react/lib/ReactDefaultInjection.js:53:1)
    at Object.getComponentClassForElement (packages/node_modules/react/lib/ReactNativeComponent.js:59:1)
    at validatePropTypes (packages/node_modules/react/lib/ReactElementValidator.js:361:1)
    at Object.ReactElementValidator.createElement (packages/node_modules/react/lib/ReactElementValidator.js:408:1)
    at Function.<anonymous> (server/routes.jsx:24:37)
    at Function.dispatchHandler (/Users/adam/projects/ssr-test/.meteor/local/isopacks/npm-container/npm/node_modules/react-router/lib/createRouter.js:381:22)
    at /Users/adam/projects/ssr-test/.meteor/local/isopacks/npm-container/npm/node_modules/react-router/lib/createRouter.js:349:29
    at /Users/adam/projects/ssr-test/.meteor/local/isopacks/npm-container/npm/node_modules/react-router/lib/Transition.js:69:9
    at /Users/adam/projects/ssr-test/.meteor/local/isopacks/npm-container/npm/node_modules/react-router/lib/Transition.js:69:9
    at Function.Transition.to (/Users/adam/projects/ssr-test/.meteor/local/isopacks/npm-container/npm/node_modules/react-router/lib/Transition.js:72:15)

@AdamBrodzinski
Copy link
Contributor

@arunoda You might find this interesting... How Netflix does their React serverside rendering. basically they're rendering a portion of the page with less data then rendering the rest on the client. This might mitigate some of the rendering CPU issues?

http://techblog.netflix.com/2015/08/making-netflixcom-faster.html

@eXon
Copy link
Contributor

eXon commented Aug 15, 2015

For those following this issue, server-rendering is working perfectly fine with the Meteor package react-router-ssr and the ReactMeteorData mixin.

@john-osullivan
Copy link

How are people using this and getting it to work correctly? I'm getting the same error as @AdamBrodzinski pointed out, trying to use the reactrouter:react-router and reactrouter:react-router-ssr packages.

@eXon
Copy link
Contributor

eXon commented Sep 30, 2015

@john-osullivan What are you trying to accomplish? With reactrouter:react-router-ssr, you don't need to renderToString or render or anything different on the client/server. It is all taken care in the package. Can you tell us more on what you are trying to do and what is not working?

@john-osullivan
Copy link

I've tried at least three different configurations: (1) installing react-router via npm, (2) using reactrouter:react-router alone, and (3) installing reactrouter:react-router-ssr and using that, none have worked correctly yet. There have been different errors in each one, so I'm unfortunately having a different issue now.

Using the third approach, where I uninstalled reactrouter:react-router as well as reactrouter:react-router-ssr, then reinstalled reactrouter:react-router-ssr, I'm currently getting a bug saying React isn't defined. The router is stored in a folder running on both the server and client. Externalify is installed and react is specified within client>lib>app.browserify.options.json, but no luck. Any idea on what should be happening here?

EDIT:
Still debugging, thought I'd keep this up to date as I check different approaches.

  • Removing the externalify transform didn't change anything, same bug.
  • Tried updating reactrouter:react-router and reactrouter:react-router-ssr, both are at their latest compatible versions. Updated react, still getting the undefined bug.

@eXon
Copy link
Contributor

eXon commented Sep 30, 2015

Can you post your code or a repo I can see your project?

Bte you shouldn't use browserify to get react or react-router. There is packages on meteor for that that arr easier to setup.

@john-osullivan
Copy link

Sure, here it is: https://github.com/john-osullivan/grup/ . The way it's currently set up, I'm not getting React myself -- I'm using Meteor's react package, and the reactrouter:react-router-ssr package to get ReactRouter. For additional context, I'm running on a Windows 10 machine.

@eXon
Copy link
Contributor

eXon commented Oct 1, 2015

@john-osullivan I seems like if you update your project to Meteor 1.2, it is wroking perfectly. I'll add a version restriction on the package because I have no intention supporting Meteor 1.1. Remove npm-container from your project, meteor update and you're good to go :)

@john-osullivan
Copy link

Thanks for the help, @eXon ! Updating to Meteor 1.2 did get rid of that error for me, although the update process recreated npm-container when it found that I had cosmos:browserify as a dependency. There must be some weirdness between our separate configurations, because I'm getting:

Error: Cannot find module 'exposify' from '/C/Users/John/AppData/Local/.meteor/packages/reactrouter_react-router/0.1.9/npm/'

Do I need to be installing and configuring exposify myself somewhere? Thanks for the help here -- I'd love to be using your code, just trying to figure out what I'm setting up incorrectly.

EDIT: Examining the full stack trace, the problem definitely seems to be in how this tool plays with cosmos:browserify:

Error: Cannot find module 'exposify' from '/C/Users/John/AppData/Local/.meteor/packages/reactrouter_react-router/0.1.9/npm/' at C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:46:17 at process (C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:173:43) at ondir (C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:188:17) at load (C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:69:43) at onex (C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:92:31) at C:\Users\John\AppData\Local\.meteor\packages\cosmos_browserify\0.8.0\plugin.CosmosBrowserify.os\npm\CosmosBrowserify\node_modules\browserify\node_modules\resolve\lib\async.js:22:47

My browserify file isn't being used to load reactrouter, so that's confusing. All it does is import classnames, which shouldn't conflict. Removing the package from my list of dependencies also didn't change anything, I get the same error.

@john-osullivan
Copy link

Hey @eXon , I just updated to version 0.2.5 of react-router-ssr and everything seems to be working as intended! No more weird browserify bugs. Still having some issues getting components to actually render, as what's being passed in seems to be an instance of ReactClass rather than the component I tried to define using React.createClass(), but that seems like an issue with how I'm doing things rather than with the library. Thanks for the help!

EDIT: To save myself the trouble of debugging how to get meteor-react-boilerplate to play nice with react-router-ssr, I'm just going to switch over to your kickstart repo! Thanks for making such a full-featured boilerplate repo, it makes things much easier for me as a Meteor beginner.

@filipenevola
Copy link
Collaborator

I'm closing this just because it's too old. We can open new issues for items that are still valid.

@meteor meteor locked and limited conversation to collaborators Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants