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

Cleaner server-side dependency injection into getInitialProps? #1117

Closed
fionawhim opened this issue Feb 13, 2017 · 20 comments
Closed

Cleaner server-side dependency injection into getInitialProps? #1117

fionawhim opened this issue Feb 13, 2017 · 20 comments

Comments

@fionawhim
Copy link
Contributor

Just wanted to raise something that I’ve noticed using the beta.

During server-side rendering (using the custom routing and renderToHTML), there are some server-side dependencies I want to make available (for example, a means of injecting requests directly into my Hapi app so that the server-side data fetching doesn’t go through a network interface). In some cases I also have data to seed an initial Redux store based on the request.

Currently I’m getting this data in by attaching custom properties to the Node Request object that’s passed in to the context.

This is fine, though a little awkward. I was raising this up in case you want to make it easier to add server-specific properties to the getInitialProps context.

@sedubois
Copy link
Contributor

If I understood right, sounds like a similar need to #978

@fionawhim
Copy link
Contributor Author

My issue is more about getting server-specific inputs into getInitialProps. I'm actually not concerned that creating the Redux store twice on the server is much of a performance hit given that the second time it's hydrated with an initial state, but I also don't know much about redux's internals.

@reel
Copy link
Contributor

reel commented Feb 14, 2017

@finneganh It's easy to add an interface with HOC to handle that. The problem is with prefetch as it might not be hydrated properly. It will only go through getInitialProps without server context. So, you will have to duplicate the server code to your getInitialProps. It gets messy. You can have a look at https://github.com/simplehub/henri/blob/master/src/client/withAuth.js and withClient.js

@fionawhim
Copy link
Contributor Author

Using a HOC to pull things off of the req is a little cleaner. I’m just wondering if adding things to req is the best way forward or if there should be another server-side mechanism for getting stuff into ctx.

@sedubois
Copy link
Contributor

@reel prefetch doesn't execute code, it only downloads it.

@thangngoc89
Copy link

I want to give a +1 for this. Currently, I'm setting up the GraphQL server in the same express instance with next.js . (I know it's bad practice but for a side project it's more than enough). Since Heroku disable local network, I had to perform some weird trick to proxy the request back to my server when doing server side rendering.

Apollo Server document stated that I should use https://github.com/af/apollo-local-query to avoid going through the HTTP stack. But doing that mean I need to bundle postgres, my schema and everything in the client bundle.

@reel
Copy link
Contributor

reel commented Feb 21, 2017

I found a solution for this... If you're using apollo or any other query that can be done server-side and client side, export your data fetching function from your React component file. Then, once you hit that route on the server, require the dist file and try to execute that function. Then, inject the result in req.data or anywhere else and render.

Reference: https://github.com/simplehub/henri/#withdata
Implementation: https://github.com/simplehub/henri/blob/master/src/henri.js#L107

I know it's not bulletproof but it works!

@unregistered
Copy link
Contributor

I know this thread is old but it seems like one way to do this is to allow passing custom data in the render() set of methods (including renderHTML()) to be given to getInitialProps(), maybe a field called serverData.

That call seems to be done here: https://github.com/zeit/next.js/blob/v3-beta/server/render.js#L63-L64

Would be a relatively simple fix if there's interest in a PR for this.

@thangngoc89
Copy link

@unregistered You can use eval

if (!process.browser) {
  const serverData = eval("require("postgrest")
}

Webpack can't resolve eval so it won't end up in client bundle

@atainter
Copy link

@thangngoc89 The idea is that data should be fetched server-side and passed though the component on render for a truly isomorphic SSR. The current method seems to be passing data through the req object in an additional data key. It's a bit roundabout and can have some implications if you're using flow types, etc.

@unregistered is suggesting that a simple code change could make this more explicit by adding a serverData key to the render method.

@unregistered
Copy link
Contributor

@thangngoc89 For those who already have a bunch of server-side data fetching being done in routes in their express apps, being able to pass the data through the nextApp.render() method would be very natural I think.

Also surely recommending consumers of your library to use eval to hack around webpack is a bit inelegant?

@thangngoc89
Copy link

@unregistered it's just my workaround for my usecase (see above). It works great for me. But you can do it another way if you like. IMHO, going through the HTTP stack is the easiest way of doing this. Same logic for client and server

@timneutkens
Copy link
Member

@unregistered I'm not highly opposed to your idea of adding another parameter. @arunoda what do you think? 😄

@arunoda
Copy link
Contributor

arunoda commented Jul 15, 2017

is suggesting that a simple code change could make this more explicit by adding a serverData key to the render method.

@unregistered @thangngoc89 I'm totally opposed of adding a server only stuff to Next.js. That's not the goal of Next.js.
We want a page to be rendered in either server or in the client at anytime.

If you want you can still fetch data and pass into your app via queryParams in the server side. Use that option. No need to add a new one.

The whole reason we are doing this to discourage you to mixing data loading stuff in the Next.js app. We want to make it minimal as possible.
So, always try to focus moving into a separate server.

If not, feel free to use custom server API and pass data via query string.

@arunoda arunoda closed this as completed Jul 15, 2017
@unregistered
Copy link
Contributor

unregistered commented Jul 15, 2017

@arunoda that's fair, I wasn't aware of that queryParams could be used this way. Perhaps this could be a candidate for an example project in /examples, since there's a handful of people who've hacked together different ways of passing data into the render() method from the server.

Regarding moving to a separate server, we have pages where data is cached in memory so we can render quickly (particularly important for an e-commerce site with a lot of traffic), and we'd rather not hop into the network stack to call localhost for the initial page render.

@arunoda
Copy link
Contributor

arunoda commented Jul 15, 2017

Regarding moving to a separate server, we have pages where data is cached in memory so we can render quickly (particularly important for an e-commerce site with a lot of traffic), and we'd rather not hop into the network stack to call localhost for the initial page render.

I think there's no reason not to do that on Next.js itself. Even you can do it inside the getInitialProps. Anyway, doing passing it via the queryParams is also a good idea.

Having a example for this case would be something great. I'd love to have it.

@reel
Copy link
Contributor

reel commented Jul 15, 2017

For custom server routes, I register a route that renders the component while injecting data in queryParams and I also register a /_data/(same route) twin route. Both routes go to the same controller but the latter, in the render function, sees the /_data/ section of url and simply res.json instead of rendering a component

@unregistered
Copy link
Contributor

@arunoda I did try that, but I had difficulty doing this in getInitialProps because the file I imported also depended on fs which would cause webpack to throw an error. @thangngoc89 said he was able to use eval() to fool webpack, and I know there's an upcoming dynamic import feature in v3, but for us (and maybe others) who have existing express servers and a bunch of middleware, it's cleaner to do in a route in Express. We especially like that Next.js is flexible enough to essentially replace something like handlebars as a view renderer.

@reel Thanks for the suggestion. I ended up kind of doing that, but opted to go with express's content negotiation feature:

res.format({
  html: () => nextApp.render(),
  json: () => res.json(data)
})

The PR with the example: #2594

Feedback appreciated 😀

@arunoda
Copy link
Contributor

arunoda commented Jul 19, 2017

@unregistered even with v3, you need to use eval. See this post.

Yep. Doing that in express looks nice and clean.

@reel
Copy link
Contributor

reel commented Jul 20, 2017

@unregistered great solution. will definitely steal it :)

timneutkens pushed a commit that referenced this issue Feb 3, 2018
* Add example on how to pass data through js api during SSR

Requested in #1117

* Use content negotiation instead of a separate route

* Codereview feedback

* Move security related test cases into a its own file.

* Removes the unused renderScript function

* Add a nerv example. (#3573)

* Add a nerv example.

* Fix for indentation/style

* Fix for name
@lock lock bot locked as resolved and limited conversation to collaborators May 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants