Allow visit to specify Rehydration/Serialization or default ClientBuilder #16227

rondale-sc · 2018-02-08T21:19:28Z

Expose as a configurable option the renderMode option. This option
would be to specify the clientBuilder to be used when Glimmer does it's
append run.

The serialization mode, used by SSR applications, is used to specify
additional markings necessary to get enough fidelity to accurately
rehydrate the DOM. For example, it would provide additional comment
nodes with codes to ensure text nodes are separated when they are rather
than merged.

The rehydration mode is specifically designed to read DOM that is
produced by the serialization mode and accurately reproduce it.

A great description of how Rehydration works can be found in this
commit:

glimmerjs/glimmer-vm@316805b

This PR allows the appropriate ElementBuilder interface to be used via
the visit API.

Current plan:

This is part of an arching plan to introduce Glimmer's
rehydration/serializtion modes to Ember proper.

There are 4 PR's that are all interwoven, of which, this is one.

Glimmer.js: Fix failing test rehydration initial content PR glimmerjs/glimmer-vm#783 (comment)
Ember.js: Allow visit to specify Rehydration/Serialization or default ClientBuilder #16227
Fastboot: Rehydration ember-fastboot/fastboot#185
EmberCLI Fastboot: Utilize rehydration serialization from glimmer ember-fastboot/ember-cli-fastboot#580

In order of need to land they are:

Glimmer.js:
glimmerjs/glimmer-vm#783 (comment)

This resolves a rather intimate API problem. Glimmer-vm expects a very
specific comment node to exist to know whether or not the rehydration
element builder can do it's job properly. If it is not found on the
first node from rootElement it throws.

In fastboot however we are certain that there will already be existant
elements in the way that will happen before rendered content.

This PR just iterates through the nodes until it finds the expected
comment node. And only throws if it never finds one.

Ember.js:
#16227

This PR modifies the visit API to allow a private _renderMode to be
set to either serialize or rehydrate. In SSR environments the
assumption (as it is here in this fastboot PR) is that we'll set the
visit API to serialize mode which ensures glimmer-vm's serialize element
builder is used to build the API.

The serialize element builder ensures that we have the necessary fidelty
to rehydrate correctly and is mandatory input for rehydration.

Fastboot:
ember-fastboot/fastboot#185

This allows enviroment variable to set _renderMode to be used in Visit
API. Fastboot must send content to browser made with the serialization
element builder to ensure rehydration can be sucessful.

EmberCLI Fastboot:
ember-fastboot/ember-cli-fastboot#580

Finally this does the fun part of disabling the current
clear-double-render instance-initializer

We first check to ensure we are in a non-fastboot environment. Then we
ensure that we can find the expected comment node from glimmer-vm's
serialize element builder. This ensures that this change will only
effect peoeple who use ember-cli-fastboot with the serialized output
from the currently experimental fastboot setup

Then we ensure ApplicationInstance#_bootSync specifies the rehydrate
_renderMode.

This is done in _bootSync this way because visit is currently not used
to boot ember applications. And we must instead set bootOptions this
way instead.

We also remove the markings for fastboot-body-start and
fastboot-body-end to ensure clear-double render instance-initializer
is never called.

josemarluedke · 2018-02-09T00:18:01Z

Awesome to see Rehydration coming to Ember!

rondale-sc · 2018-02-09T14:31:32Z

To assuage any fears of file size bloat this takes ember.min.js (gzipped) from 115.73kb to 115.89. Which should definitely be addressed, but I shouldn't think be a blocker.

rwjblue · 2018-02-09T14:38:39Z

To assuage any fears of file size bloat this takes ember.min.js (gzipped) from 115.73kb to 115.89. Which should definitely be addressed, but I shouldn't think be a blocker.

Agreed.

rondale-sc · 2018-02-09T16:03:36Z

Relevant to this ticket: glimmerjs/glimmer-vm#780

rondale-sc · 2018-02-09T21:06:01Z

After discussing this, it was decided that the configuration for renderMode should be a private var to avoid muddying the API surface. Updated the PR accordingly

rwjblue · 2018-02-13T20:39:31Z

looks like this needs a rebase

rondale-sc · 2018-02-13T21:01:14Z

@rwjblue Rebased. I'm still running tests locally, but should be gtg

rondale-sc · 2018-02-14T12:17:13Z

Looks like I need to dig into this one. Working on it now.

rwjblue

Looking really nice! A few more things to cleanup...

After the initial serialize render we should grab the actual div element so that we can ensure it is "rehydrated"
Add additional assertions that confirm the element is exactly the initial element
Add additional assertion that confirm the template isn't rendered into the root element twice...

rwjblue · 2018-02-15T20:24:04Z

packages/ember-application/tests/system/visit_test.js

+
+    ENV._APPLICATION_TEMPLATE_WRAPPER = false;
+
+    return this.runTask(() => {


we should remove this runTask (otherwise this makes a nested run loop and we shouldn't need that)

rwjblue · 2018-02-15T20:32:12Z

packages/ember-application/tests/system/visit_test.js

+        .then((instance) => {
+          assert.equal(
+            instance.rootElement.firstChild.nodeValue,
+            '%+b:0%',


I don't really like the coupling to the internal / private mechanism that Glimmer's serialize mode renderer uses internally (generally speaking I'd much prefer to test the results rather than how it works internally) but I realize this is somewhat of a chicken or the egg situation.

Thoughts?

I totally agree that I'd prefer not to couple so tightly to the internal implementation. Though to ensure we are actually doing some work this seems to be necessary.

I'd like in the future to expose the leading comment node in the serializer builder in glimmer as an export so we don't have to hardcode this value. Perhaps that could be a next step?

rwjblue · 2018-02-15T20:33:47Z

packages/ember-application/tests/system/visit_test.js

+        rootElement,
+        _renderMode: 'rehydrate'
+      };
+      this.application.visit('/', bootOptions).then(instance => {


We should be able to do this.visit again (since you are properly destroying/clearing it above).

@rwjblue

ClientBuilder Expose as a configurable option the `renderMode` option. This option would be to specify the clientBuilder to be used when Glimmer does it's append run. The serialization mode, used by SSR applications, is used to specify additional markings necessary to get enough fidelity to accurately rehydrate the DOM. For example, it would provide additional comment nodes with codes to ensure text nodes are separated when they are rather than merged. The rehydration mode is specifically designed to read DOM that is produced by the serialization mode and accurately reproduce it. A great description of how Rehydration works can be found in this commit: glimmerjs/glimmer-vm@316805b This PR allows the appropriate ElementBuilder interface to be used via the `visit` API. Additional Work: - Update Fastboot to pass the appropriate `renderMode` flag such that it generates the serialization format DOM - Update ember-cli-fastboot instance-initializer to not do a double boot and instead configure it to use the rehydration `renderMode` - See more information here: https://github.com/ember-fastboot/ember-cli-fastboot/blob/master/addon/instance-initializers/clear-double-boot.js Open Questions: - Is the `renderMode` flag Public API - Should this be put behind a feature flag - @rwjblue noted that this technically would be a bug fix as it would remove the double render problem with SSR ember apps

This is meant to be a temporary test. Want a more robust test that doesn't rely on the actual serialization format of glimmer's serializer builder

Formerly we were asserting on a string we manually created with knowledge of the glimmer-vm encoding for both serialization and rehydration. This test now generates an application instance with the serialize method and asserts that the first node is a node we know is set by glimmer-vm serializer Then we destroy that instance and create a new instance with the rehydrate _renderMode that uses it to rehydrate the dom. Thus we go through a closer to real-world scenario of serialization in an SSR visit with _renderMode set appropriately, through visit with an instance set to rehydrate the SSR generated content.

the application

rondale-sc · 2018-02-22T15:59:16Z

packages/ember-application/tests/system/visit_test.js

+          _renderMode: 'rehydrate'
+        };
+
+        this.application.visit('/', bootOptions).then(instance => {


@rwjblue turns out this is actually necessary. When you remove the this.application and just call visit directly we get an error saying that you tried to use an object that is destroyed (sorry I don't have the exact wording of the error) . Either way nbd.

Seems fine, but I still don't like it 😝

rondale-sc · 2018-02-22T16:00:15Z

packages/ember-application/lib/system/application-instance.js

+      @default false
+      @private
+    */
+    this._renderMode = options._renderMode;


This was recently changed and I think the commit history of this PR is now a little inaccurate to what actually happened. I suspect we'll sqaush soon anyways, so I'm not worried about it, but if it is a concern I can flesh out the commit history.

This is part of an arching plan to introduce Glimmer's rehydration/serializtion modes to Ember proper. There are 4 PR's that are all interwoven, of which, this is one. In order of need to land they are: Glimmer.js: glimmerjs/glimmer-vm#783 (comment) This resolves a rather intimate API problem. Glimmer-vm expects a very specific comment node to exist to know whether or not the rehydration element builder can do it's job properly. If it is not found on the first node from `rootElement` it throws. In fastboot however we are certain that there will already be existant elements in the way that will happen before rendered content. This PR just iterates through the nodes until it finds the expected comment node. And only throws if it never finds one. --- Ember.js: emberjs/ember.js#16227 This PR modifies the `visit` API to allow a private _renderMode to be set to either serialize or rehydrate. In SSR environments the assumption (as it is here in this fastboot PR) is that we'll set the visit API to serialize mode which ensures glimmer-vm's serialize element builder is used to build the API. The serialize element builder ensures that we have the necessary fidelty to rehydrate correctly and is mandatory input for rehydration. --- Fastboot: ember-fastboot#185 This allows enviroment variable to set _renderMode to be used in Visit API. Fastboot must send content to browser made with the serialization element builder to ensure rehydration can be sucessful. --- EmberCLI Fastboot: ember-fastboot/ember-cli-fastboot#580 Finally this does the fun part of disabling the current clear-double-render instance-initializer We first check to ensure we are in a non-fastboot environment. Then we ensure that we can find the expected comment node from glimmer-vm's serialize element builder. This ensures that this change will only effect peoeple who use ember-cli-fastboot with the serialized output from the currently experimental fastboot setup Then we ensure `ApplicationInstance#_bootSync` specifies the rehydrate _renderMode. This is done in `_bootSync` this way because visit is currently not used to boot ember applications. And we must instead set bootOptions this way instead. We also remove the markings for `fastboot-body-start` and `fastboot-body-end` to ensure clear-double render instance-initializer is never called.

This is part of an arching plan to introduce Glimmer's rehydration/serializtion modes to Ember proper. There are 4 PR's that are all interwoven, of which, this is one. - [ ] Glimmer.js: glimmerjs/glimmer-vm#783 (comment) - [ ] Ember.js: emberjs/ember.js#16227 - [ ] Fastboot: ember-fastboot/fastboot#185 - [ ] EmberCLI Fastboot: ember-fastboot#580 In order of need to land they are: Glimmer.js: glimmerjs/glimmer-vm#783 (comment) This resolves a rather intimate API problem. Glimmer-vm expects a very specific comment node to exist to know whether or not the rehydration element builder can do it's job properly. If it is not found on the first node from `rootElement` it throws. In fastboot however we are certain that there will already be existant elements in the way that will happen before rendered content. This PR just iterates through the nodes until it finds the expected comment node. And only throws if it never finds one. --- Ember.js: emberjs/ember.js#16227 This PR modifies the `visit` API to allow a private _renderMode to be set to either serialize or rehydrate. In SSR environments the assumption (as it is here in this fastboot PR) is that we'll set the visit API to serialize mode which ensures glimmer-vm's serialize element builder is used to build the API. The serialize element builder ensures that we have the necessary fidelty to rehydrate correctly and is mandatory input for rehydration. --- Fastboot: ember-fastboot/fastboot#185 This allows enviroment variable to set _renderMode to be used in Visit API. Fastboot must send content to browser made with the serialization element builder to ensure rehydration can be sucessful. --- EmberCLI Fastboot: ember-fastboot#580 Finally this does the fun part of disabling the current clear-double-render instance-initializer We first check to ensure we are in a non-fastboot environment. Then we ensure that we can find the expected comment node from glimmer-vm's serialize element builder. This ensures that this change will only effect peoeple who use ember-cli-fastboot with the serialized output from the currently experimental fastboot setup Then we ensure `ApplicationInstance#_bootSync` specifies the rehydrate _renderMode. This is done in `_bootSync` this way because visit is currently not used to boot ember applications. And we must instead set bootOptions this way instead. We also remove the markings for `fastboot-body-start` and `fastboot-body-end` to ensure clear-double render instance-initializer is never called.

rwjblue · 2018-02-23T22:12:23Z

packages/ember-application/tests/system/visit_test.js

+          _renderMode: 'rehydrate'
+        };
+
+        this.application.visit('/', bootOptions).then(instance => {


Seems fine, but I still don't like it 😝

This is part of an arching plan to introduce Glimmer's rehydration/serializtion modes to Ember proper. There are 4 PR's that are all interwoven, of which, this is one. - [ ] Glimmer.js: glimmerjs/glimmer-vm#783 (comment) - [ ] Ember.js: emberjs/ember.js#16227 - [ ] Fastboot: ember-fastboot/fastboot#185 - [ ] EmberCLI Fastboot: ember-fastboot#580 In order of need to land they are: Glimmer.js: glimmerjs/glimmer-vm#783 (comment) This resolves a rather intimate API problem. Glimmer-vm expects a very specific comment node to exist to know whether or not the rehydration element builder can do it's job properly. If it is not found on the first node from `rootElement` it throws. In fastboot however we are certain that there will already be existant elements in the way that will happen before rendered content. This PR just iterates through the nodes until it finds the expected comment node. And only throws if it never finds one. --- Ember.js: emberjs/ember.js#16227 This PR modifies the `visit` API to allow a private _renderMode to be set to either serialize or rehydrate. In SSR environments the assumption (as it is here in this fastboot PR) is that we'll set the visit API to serialize mode which ensures glimmer-vm's serialize element builder is used to build the API. The serialize element builder ensures that we have the necessary fidelty to rehydrate correctly and is mandatory input for rehydration. --- Fastboot: ember-fastboot/fastboot#185 This allows enviroment variable to set _renderMode to be used in Visit API. Fastboot must send content to browser made with the serialization element builder to ensure rehydration can be sucessful. --- EmberCLI Fastboot: ember-fastboot#580 Finally this does the fun part of disabling the current clear-double-render instance-initializer We first check to ensure we are in a non-fastboot environment. Then we ensure that we can find the expected comment node from glimmer-vm's serialize element builder. This ensures that this change will only effect peoeple who use ember-cli-fastboot with the serialized output from the currently experimental fastboot setup Then we ensure `ApplicationInstance#_bootSync` specifies the rehydrate _renderMode. This is done in `_bootSync` this way because visit is currently not used to boot ember applications. And we must instead set bootOptions this way instead. We also remove the markings for `fastboot-body-start` and `fastboot-body-end` to ensure clear-double render instance-initializer is never called.

rondale-sc force-pushed the utilize-rehydration-serialization-from-glimmer branch from d53b2e8 to 994e154 Compare February 8, 2018 21:48

rwjblue changed the title ~~Allow visit to specify Rehydration/Serialization or default ClientBuilder~~ [WIP] Allow visit to specify Rehydration/Serialization or default ClientBuilder Feb 9, 2018

rondale-sc force-pushed the utilize-rehydration-serialization-from-glimmer branch from c6e16a3 to 26f9841 Compare February 13, 2018 21:00

rondale-sc force-pushed the utilize-rehydration-serialization-from-glimmer branch 3 times, most recently from e633523 to 522a43b Compare February 14, 2018 20:03

rwjblue reviewed Feb 15, 2018

View reviewed changes

rondale-sc added 7 commits February 22, 2018 10:47

Make _renderMode private

27f4e2d

Ensure correct signature for renderMain

882bbdd

update serialized format to new glimmer-vm output

0627ae6

This is meant to be a temporary test. Want a more robust test that doesn't rely on the actual serialization format of glimmer's serializer builder

Remove unnecessary runTask

c6386c0

this.visit since a new instance will be allocated because we destroyed

04ad41d

the application

rondale-sc force-pushed the utilize-rehydration-serialization-from-glimmer branch from 8501563 to 04ad41d Compare February 22, 2018 15:57

rondale-sc commented Feb 22, 2018

View reviewed changes

This was referenced Feb 23, 2018

Utilize rehydration serialization from glimmer ember-fastboot/ember-cli-fastboot#580

Merged

Rehydration ember-fastboot/fastboot#185

Merged

Fix failing test rehydration initial content PR glimmerjs/glimmer-vm#783

Merged

rwjblue approved these changes Feb 23, 2018

View reviewed changes

rondale-sc mentioned this pull request Feb 24, 2018

Export isRehydrationComment for rehydration/serialization concerns glimmerjs/glimmer-vm#787

Closed

rwjblue changed the title ~~[WIP] Allow visit to specify Rehydration/Serialization or default ClientBuilder~~ Allow visit to specify Rehydration/Serialization or default ClientBuilder Feb 25, 2018

rwjblue merged commit 8be1a9d into emberjs:master Feb 25, 2018


		ENV._APPLICATION_TEMPLATE_WRAPPER = false;

		return this.runTask(() => {

Uh oh!

Allow visit to specify Rehydration/Serialization or default ClientBuilder #16227

Allow visit to specify Rehydration/Serialization or default ClientBuilder #16227

Uh oh!

Conversation

rondale-sc commented Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josemarluedke commented Feb 9, 2018

Uh oh!

rondale-sc commented Feb 9, 2018 • edited by rwjblue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rwjblue commented Feb 9, 2018

Uh oh!

rondale-sc commented Feb 9, 2018

Uh oh!

rondale-sc commented Feb 9, 2018

Uh oh!

rwjblue commented Feb 13, 2018

Uh oh!

rondale-sc commented Feb 13, 2018

Uh oh!

rondale-sc commented Feb 14, 2018

Uh oh!

rwjblue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rondale-sc commented Feb 8, 2018 •

edited

Loading

rondale-sc commented Feb 9, 2018 •

edited by rwjblue

Loading