Separate Application responsibilities

[tomdale]

As @chadhietala noted in #31, as the Glimmer VM has continued to expand its platform of capabilities, supporting the matrix of configuration options is becoming increasingly difficult.

Mode	Client	Server
DOM Rehydration	X
DOM Building	X
String Building (SEO)		X
String Building (Rehydration)		X
Lazy Compilation	X	X
Eager Compilation	X	X
Synchronous Rendering	X	X
Async/Incremental Rendering	X	X

Adding to the difficulty is the fact that some of the "modes" carry much more code than others. We need an API that makes configuration as simple as possible, while ensuring that the final payload only includes code for modes that are actually used.

This commit breaks the majority of the Application's responsibilities into three composable objects:

1. The Loader, which may load and/or compile an application's templates.
2. The Builder, which determines how template output is built (e.g. building DOM).
3. The Renderer, which schedules rendering work.

Examples

Today

Today's Glimmer.js app has the following characteristics:

1. Lazy, in-browser bytecode compilation.
2. Non-rehydrating, client-side rendering.
3. Synchronous initial render.

In the API proposed here, the bootstrapping code would look like this:

import Application, { DOMBuilder, SyncRenderer, RuntimeLoader } from '@glimmer/application';
import Resolver, { BasicModuleRegistry } from '@glimmer/resolver';
import moduleMap from '../config/module-map';
import resolverConfiguration from '../config/resolver-configuration';

let moduleRegistry = new BasicModuleRegistry(moduleMap);
let resolver = new Resolver(resolverConfiguration, moduleRegistry);

let element = document.getElementById('app');

let app = new Application({
  builder: new DOMBuilder({ element }),
  renderer: new SyncRenderer(),
  loader: new RuntimeLoader(resolver)
});

app.boot();

This is very similar to the code generated by the blueprint today. The only change is that, instead of passing the resolver and element to the Application constructor directly, we wrap them in a RuntimeLoader and DOMBuilder, respectively. We then pass the RuntimeLoader, DOMBuilder and SyncRenderer to the Application constructor.

So far, we've added a little bit of boilerplate for the same semantics. Not much of an improvement! Let's see where this gets more interesting.

Bytecode

Imagine we now want to move the final bytecode compilation step out of the browser and into our build process. To do this, we will compile our templates into a single binary .gbx file. Additionally, we want to incrementally perform the initial render, so lower-end devices stay responsive even if our DOM is large.

Now we can reconfigure our application like this:

import Application, { DOMBuilder, AsyncRenderer, BytecodeLoader } from '@glimmer/application';

import data from './__compiled__/data';
let bytecode = fetch('./__compiled__/templates.gbx')
  .then(req => req.arrayBuffer());

let element = document.getElementById('app');

let app = new Application({
  builder: new DOMBuilder({ element }),
  renderer: new AsyncRenderer(),
  loader: new BytecodeLoader({ data, bytecode })
});

app.boot().then(() => {/* ... */});

With very similar code to the example above, we've now radically changed how our application behaves. It will fetch the binary bytecode and pass it to the BytecodeLoader, which is much smaller and faster than the RuntimeLoader. Because we pass in the AsyncRenderer, the initial render will be batched via requestIdleCallback so the browser stays responsive, no matter how many frames it takes to render the whole page.

Additional Examples

For additional examples of how this API may be used to fine-tune the performance of a Glimmer.js application, see this gist.

Additional Changes

This PR is intentionally scoped as small as possible, with the hope that it can be merged early to avoid a long-lived branch. There are several shortcomings in the implementation here that I intend to address before we release:

1. Given that renderers can be sync or async, we should move all user-facing Application methods to be async and return promises (boot() etc.).
2. To ease getting the tests passing, the Application provides a default set of renderer, loader and builder if they are not passed to the constructor. We will want to update the test harness to pass the appropriate configuration, and throw an exception if the required objects are not passed to the Application constructor.

[knownasilya]

Should the error at least be logged?

[tomdale]

execa throws an exception if the return value is non-zero, so you end up with a useless stack trace every time the tests fail. Since we pipe stderr to the parent stderr you will see the error messages anyway. You're right though that we should probably check to verify that the error isn't caused by a bad path or something and is really a non-zero exit code.

[knownasilya]

Thanks for the context. Even if nothing is handled, a comment would probably help in regards to why the error is ignored 👍

[rwjblue]

@tomdale - Can you confirm that failing node tests properly fail the build now? My concern is that swallowing this error, will "eat" the non-zero exit code and the tests would seen as "passing" (due to process exiting with 0 exit code) when they actually failed...

[chadhietala]

Would like to not repeat what is done in the VM when it comes to swallowing the error.

[rwjblue]

haha, indeed!

[tomdale]

@knownasilya The release version of execa hides all information from you in the case of a non-zero exit code in the sync case, which is really unfortunate. It looks like a PR was accepted 6 days ago to address this but hasn't been released yet.

@rwjblue In my tests, running yarn test with a failing Node test produces an exit code of 1, so I think we're good. I would expect Testem to treat TAP output that contained not ok as a failure, which it seems to be doing.

[chadhietala]

I think we might need to guard all of this with if (result) { ... }

[tomdale]

The guard is in SyncRenderer; the Application no longer knows about the render result.

[chadhietala]

Remove

[tomdale]

👍

[chadhietala]

Re-render is synchronous but render is async? Seems like these should be balanced or at least check for the balancing.

[tomdale]

In practice, rerender cannot be async at the moment. The interface for rerender() already supports a Promise return type, but there aren't any implementations that can test this path yet.

[chadhietala]

This is a Cursor and the type is exposed from @glimmer/runtime as such. If you want a type alias for the same thing just extend it.

[tomdale]

@chadhietala Deny, Cursor does not allow nextSibling to be undefined. I don't think we want to make users have to explicitly set nextSibling to null every time.

[chadhietala]

It is an Option. In the constructor just do { element, nextSibling = null }

[knownasilya]

Woot!