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

Exploring incremental rendering as the default #481

Closed
matthewp opened this Issue Feb 6, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@matthewp
Contributor

matthewp commented Feb 6, 2018

This issue is to explore what it would take to make incremental rendering the default. I'll be testing in some projects and writing notes here on things like bugs, UX, etc.

Overview

Incremental rendering is really 2 things. One is a mutation stream renderer and the other is a reattacher. I would rate the stability of these things as:

  • Mutation Stream: 8/10 - (fairly stable, still find bugs when trying in new apps)
  • iframe Reattacher: 4/10 - Not so stable.
    • Styles not currently showing in the iframe.
    • If HTML is wrong in any way the reattachment happens too quickly.

Issues

  • - Currently we do not support H2 (require('http2')). Instead we use a fork of node-spdy which is not really maintained. We should definitely switch to the build-in module.
  • - We should make it easier to debug IR instructions: #483
  • - The iframe idea causes images to be loaded twice (if uncached): #401

UX

  • - --cert and --key arguments are required to start an H2 server. Would it maybe be possible to include a default key/cert for development?

Apps

place-my-order

  • - Initial styles are not part of the iframe overlay.
  • - Detachment doesn't happen
@matthewp

This comment has been minimized.

Contributor

matthewp commented Feb 7, 2018

Browser support issues

  • HTTP2: 🚫 IE11 (requires TLS in supporting browsers)
  • <iframe srcdoc="<html>....</html>"></iframe>: 🚫 Edge
  • fetch(): 🚫 IE11
  • ReadableStream: 🚫 Firefox
@matthewp

This comment has been minimized.

Contributor

matthewp commented Feb 7, 2018

I found a few bugs when looking into this issue with using IR in place-my-order. I'm going to go ahead and fix those bugs.

@justinbmeyer

This comment has been minimized.

Contributor

justinbmeyer commented Feb 16, 2018

(2 weeks)

  • need to use built in node H2 module
  • properly setup fallback for lack of browser support
@matthewp

This comment has been minimized.

Contributor

matthewp commented Feb 19, 2018

Outstanding issues

In order to proceed with incremental rendering there are 2 main blocking issues. They are:

Use Node.js' http2 module

It's a little bit different from the http module, but there is a compatibility api so it might not be too bad to use it.

Figure out how to handle SSL certs for development mode

Setting up an SSL certificate for local development is a pain. Even for experienced developers. For inexperienced users it's probably the difference between using and not using a framework.

So we need a way to provide one by default, or to create one at build-time or something.

Additionally when you create a self-signed certificate you get an ugly error message in browsers that you have to know how to ignore. This flow needs to be improved.

@matthewp

This comment has been minimized.

Contributor

matthewp commented Jul 23, 2018

I'm going to begin working on this soon. This is broken up into 3 main things:

  • Upgrade to use Node's builtin http2 module.
  • Breaking up dom-patch into separate libraries.
  • Revisit the reattachment part, how can we do this better?

I'm going to start with dom-patch since it's pretty straight forward, it will be broken into separate libraries that do one part:

  • done-mutation-observer - A MO polyfill for can-simple-dom and jsdom.
  • done-mutation-serialize - Takes MutationRecords and serializes them.
  • done-mutation-patch - Takes the serialized form from done-mutation-serialize and turns them into patches that can be applied to a DOM.

dom-patch will be deprecated once this is complete, and no longer used in done-ssr.

@matthewp

This comment has been minimized.

Contributor

matthewp commented Oct 11, 2018

Just need to toggle it to become the default. I'm updating the estimate based on that.

@matthewp

This comment has been minimized.

Contributor

matthewp commented Nov 1, 2018

Closed by #598

@matthewp matthewp closed this Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment