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

Example: Passing data from server through API #2594

Merged
merged 9 commits into from
Feb 3, 2018

Conversation

unregistered
Copy link
Contributor

@unregistered unregistered commented Jul 19, 2017

As requested by @arunoda here is an example of how to pass data in the js api through the query params. Relates to Issue #1117

I opened this against the master branch but let me know if it should go against v3 beta

console.log('Express is handling request')
const itemData = api.getItem()

res.format({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is an example, may be it's better to do this inside the "server.js" to keep it simple and short.
And I think we don't need to use res.format as well.

do something like this:

server.get('/item', (req, res) => {
  const itemData = api.getItem()
  return app.render(req, res, '/item', { itemData })
})

Copy link
Contributor Author

@unregistered unregistered Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The res.format is to support isomorphic rendering of that page so it works when linked to. I could also mount 2 separate routes, one at /item which renders the html, another at maybe /_data/item which sends the json. But I've found that doing it in the same route method is more concise. Seems to me that it would be necessary, but let me know if I'm misunderstanding something.

I can move the route to server.js though, and leave how they break things off up to the reader.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then may be you need to expose a different route for the data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the CR. Is that what you had in mind?

@arunoda
Copy link
Contributor

arunoda commented Jul 19, 2017

This is great. Could you send this against the v3-beta branch?

@timneutkens timneutkens changed the base branch from master to v3-beta July 19, 2017 12:09
@unregistered unregistered changed the base branch from v3-beta to master July 19, 2017 17:58
@unregistered unregistered changed the base branch from master to v3-beta July 19, 2017 17:59
@unregistered
Copy link
Contributor Author

unregistered commented Jul 19, 2017

@arunoda here it is against v3-beta, though there's another change in package.json that you'd have to verify. If it's not I can open a new PR with v3-beta on my fork. Edit: nevermind, it looks like that will go away with a merge conflict resolve.

@unregistered
Copy link
Contributor Author

unregistered commented Jul 20, 2017

Hi @arunoda I've noticed a downside to doing it this way, which is that next inserts all the data passed into query into __NEXT_DATA__ in the SSR'd output:

<script>
__NEXT_DATA__ = {"props":{"item":{"title":"Now","subtitle":"Realtime global deployments","seller":"Zeit"}},"pathname":"/item","query":{"itemData":{"title":"Now","subtitle":"Realtime global deployments","seller":"Zeit"}},"buildId":1500535262707,"buildStats":null,"assetPrefix":"","err":null}

This can be undesirable when injecting a large amount of data that's used incidentally during rendering, or passing sensitive data the user should not be able to see. Any advice?

@unregistered
Copy link
Contributor Author

Hi @arunoda do you have any thoughts on my latest comment regarding leaking data in the HTML?

@timneutkens timneutkens changed the base branch from v3-beta to master August 27, 2017 20:17
@pruhstal
Copy link

pruhstal commented Nov 6, 2017

@unregistered I think this is because:

https://github.com/zeit/next.js/blob/1424b84c98e5e8e2efc279d1790c49c970f21eca/server/render.js#L98

is calling this: https://github.com/zeit/next.js/blob/canary/lib/utils.js#L49

Which checks to see if there's a getInitialProps method attached on the component.

If it is it'll spread those props into __NEXT_DATA__.

If you could somehow call res.finished right after this:

https://github.com/zeit/next.js/pull/2594/files#diff-3f14957c7ec2d722cdf540ed20059dd4R19

it should hopefully not inject those props (but I haven't tried it yet) per this:

https://github.com/zeit/next.js/blob/1424b84c98e5e8e2efc279d1790c49c970f21eca/server/render.js#L66

If not, then maybe @arunoda knows, however, this is a good example and I'd like to see it get added!

EDIT: this looks like it does something similar: https://github.com/zeit/next.js/blob/18f8ab392ae2a93140df2545f537c544c1b88241/examples/with-apollo-auth/lib/withData.js#L42

arunoda and others added 2 commits January 13, 2018 22:45
* Add a nerv example.

* Fix for indentation/style

* Fix for name
@danielbayerlein
Copy link
Contributor

Any news?

@unregistered
Copy link
Contributor Author

@danielbayerlein we currently use a variation of this on our site, except instead of passing data through query, we mutate the req object to attach ssrData. This has the benefit of avoiding the serialization issue mentioned above.

@timneutkens timneutkens changed the base branch from master to canary February 2, 2018 19:19
@timneutkens
Copy link
Member

Will merge when the tests pass 🙏

@timneutkens timneutkens merged commit 7afc008 into vercel:canary Feb 3, 2018
@danielbayerlein
Copy link
Contributor

@timneutkens Thank you!!! 💚

@lock lock bot locked as resolved and limited conversation to collaborators Feb 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants