Themes: SSR testing branch #846
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ehg
added
[Type] Question
[Feature Group] Appearance & Themes
| @@ -172,12 +173,27 @@ function boot() { | |||
| if ( config.isEnabled( 'oauth' ) ) { | |||
| LoggedOutLayout = require( 'layout/logged-out-oauth' ); | |||
| } else { | |||
| LoggedOutLayout = require( 'layout/logged-out' ); | |||
| LoggedOutLayout = require( 'layout/themes-logged-out' ); | |||
There was a problem hiding this comment.
Since themes-logged-out is a 'pure' render tree, we can't render into sections directly in our controllers. How do we update our routing & controllers to take this into account, and how do we make this generically usable?
2 tasks
Render themes/main on server - Inject i18n mixin - Init i18n - Render themes/main Allow localStorage on node Add primary/tertiary elements to ThemesLoggedOutLayout Use ThemesSelection for the logged-out design/ route Stub things with NormalModuleReplacementPlugin Make InfiniteList not depend on the DOM
It doesn't crash the browser any more, and there are no markup difference warnings(!) This commit needs tidying up, and contains some extraneous stuff, but: - Trees must be the same for client and server, so only render themes-selection in client for logged out --HACK-- * We may have to consider starting SSR logged out only on /themes. Routing & the current way of replacing whole sections in the DOM is incompatible. - We can't use React to render the whole `document`, so let's stick with Jade for now. - Components with _instance vars updated to only update the instance id in componentDidMount, otherwise the server will increment on each request. - Spinner moved to components for now, stubbing is only useful for components that don't get rendered on the first pass. - `Main` moved to shared, and put directly in ThemesLoggedOutLayout - Disabled the controller from rendering on client /design - otherwise boom, crash.
Moved most /design specific handling out of client/boot/index.js so that other logged out routes will work as normal. Layout parameters are being passed to the layout component via setState(), which could be causing the problems with React failing to reconcile that component with the server-rendered version.
ehg
force-pushed
the
try/node-webpack-stubbing
branch
from
0aba936 to
351f69e
Compare
Until we have a solution that allows rendering of entire React layout depending on the route, move the necessary themes-layout props inside the themes-logged-out component so that the client can immediately render indentically to the server. (Previously, the first client render of the themes-logged-out component was empty.)
(Since Spinner comopnent has already been moved to shared). This fixes a difference between the server and client renders.
If we're using our own layout and echewing routing for now, we can get rid of some code in pages/index.js We can also employ a poor man's tier routing, using a regex hack and 'dynamic' tier prop insertion. /cc @seear
Removes a dependency for logged-out server-side rendering.
Themes search uses the url-search mixin, which causes the controller to pass the search query down as a prop whenever it changes. Since themes-logged-out is not using a controller, that does not happen. This change sets state on search to cause a re-render, much like tier selection.
|
Closing for now since no activity in 6 months — feel free to re-open and continue discussing when the time allows or interest picks back up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE
This PR is a playground for testing Server Side Rendering on the Showcase. It's full of hacks and test code, and is only intended for exploration of techniques, and to top-down test recently merged SSR related PRs.