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 revamp #8841

Merged

Conversation

benjamn
Copy link
Contributor

@benjamn benjamn commented Jun 23, 2017

To support server-side rendering of any kind, it would be ideal if there was a way for server code to inject rendered HTML into the boilerplate HTML generated by packages like static-html. Until now, that hasn't been possible, since the only mechanism for adding dynamic HTML to the initial HTTP response was to modify request.dynamicHead and/or request.dynamicBody.

With this new API, it should now be possible to register callbacks that (for example) parse boilerplate.baseData.body, find DOM nodes by id, render HTML fragments, inject the fragments into the DOM, and set boilerplate.baseData.body to the resulting HTML string.

I previously thought this would require changes to the boilerplate-generator package, and thus we had to wait for #8820 to be finished, but it turns out we can intercept boilerplate.baseData before calling boilerplate.toHTML, with only the changes in this PR.

I also took this opportunity to modernize the webapp package to use more ECMAScript 2015+ features, so it may be easier to review this PR commit-by-commit.

@benjamn benjamn added this to the Release 1.5.1 milestone Jun 23, 2017
@benjamn benjamn self-assigned this Jun 23, 2017
@benjamn benjamn force-pushed the WebAppInternals.registerBoilerplateDataCallback branch from c02776d to 8cf3de9 Compare June 23, 2017 21:57
Ben Newman added 3 commits June 23, 2017 19:00
This API allows registering callbacks that have the opportunity to modify
boilerplate.baseData on each request, which will be useful for
implementing server-side rendering.

The code in question behaves the same as before if there are no callbacks
registered, so this change should be completely backwards compatible.
@benjamn benjamn force-pushed the WebAppInternals.registerBoilerplateDataCallback branch from 8cf3de9 to a2bb182 Compare June 23, 2017 23:01
@benjamn
Copy link
Contributor Author

benjamn commented Jun 24, 2017

Should the callback be able to return a Promise? Is that feasible without using Fibers to await the result synchronously?

@zimme
Copy link
Contributor

zimme commented Jun 24, 2017

I guess getBoilerplate() would have to return a promise in that case also, so I guess we would have to use Fibers or some promises to wait for all callbacks to resolve before sending it down the pipe?

I think it might be doable without fibers if we use promises all the way to the middleware?

Ben Newman added 2 commits June 24, 2017 18:21
This is a bug that will be fixed by @stevenhao's boilerplate-generator
refactoring (#8820), but I need it fixed now :)
Copy link
Contributor

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

This is awesome @benjamn! I've been playing around with these changes a bit, and things are working well!

@@ -1,6 +1,6 @@
Package.describe({
summary: "Serves a Meteor app over HTTP",
version: '1.3.16'
version: '1.3.17'
});

Npm.depends({connect: "2.30.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to these changes, but we should really consider jumping to connect 3.x at some point.

@benjamn benjamn changed the title Implement WebAppInternals.registerBoilerplateDataCallback. Server-side rendering revamp Jun 26, 2017
Copy link
Contributor

@abernix abernix left a comment

Choose a reason for hiding this comment

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

Aside from what seem to be spurious css hot code push test failures, this LGTM.

@benjamn
Copy link
Contributor Author

benjamn commented Jun 26, 2017

@abernix @hwillson Should we go ahead and merge this into release-1.5.1 and continue working/debugging there?

@abernix
Copy link
Contributor

abernix commented Jun 26, 2017

@benjamn I think the failures are all related but not related to any of these actual PRs. So, yes.

@benjamn benjamn merged commit 49f566d into release-1.5.1 Jun 26, 2017
@zimme zimme mentioned this pull request Jun 27, 2017
@marcoschwartz
Copy link

Sounds like a great feature ! Does this apply to blaze or it's only for react ? Thx

@msavin
Copy link

msavin commented Jul 5, 2017

+1 - would love to see this working with Blaze, or even just Spacebars since we do not need the re-rendering/etc.

@rgnevashev
Copy link

this is a great feature... but:

  1. How can I redirect inside the onPageLoad func? See react-router v4 SSR example
  2. How can I obtain userId in my components on server side? My App depends on currentUser and I want obtain userId on server side and pass to my App entry point component...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants