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

Asynchronous render #645

Open
diffcunha opened this issue Mar 14, 2018 · 16 comments · May be fixed by #646
Open

Asynchronous render #645

diffcunha opened this issue Mar 14, 2018 · 16 comments · May be fixed by #646

Comments

@diffcunha
Copy link

diffcunha commented Mar 14, 2018

Rendering in Choo is a completely synchronous process, thought this makes things way simpler there are a few downsides from 2 perspectives:

  • CPU - Rendering complex components might block the main loop affecting the overall experience
  • I/O - It's not possible to make the render wait for asynchronous I/O events such as data loading

I believe the later is the most relevant when it comes to SSR. One of the biggest issues of sync SSR is related to data loading and code-splitting. There are several workaround strategies to deal with async I/O on a sync rendering process, most of them require a first-pass render to load or collect something you can wait on and signal when the data is available to read synchronously. Besides being a waste of CPU cycles it's also not very elegant and adds unnecessary complexity to the code. One could just not have SSR but being able to have truly isomorphic or universal app is a requirement for most content-driven apps.

When I think of async rendering I think of composing the view using sync or async components and then one of two things can happen:

  • Wait until all the components are resolved and then apply node diffing
  • Apply node diffing as components resolve

Ideally both scenarios should be allowed, giving that choice to the developer, but I believe the first one is easier to start with given the current Choo architecture. Overall it should look somehow like this:

const h = require('choo/html')

const syncComponent = (state, emit) => h`
  <p>${state.label}</p>
`

const asyncComponent = async (state, emit) => {
  const data = await asyncDataFetch(state.id)
  return h`
    <p>${data}</p>
  `
}

const view = (state, emit) => h` 
  <div>
    ${syncComponent(state, emit)}
    ${asyncComponent(state, emit)}
  </div>
`

Given that there are a few necessary steps:

  1. Make _prerender function on Choo await for the promise returned by the render to resolve
  2. Make bel/nanohtml also accept promises
  3. Make mount start, toString methods on Choo async since they rely on _prerender

There are a few open-ended questions at the moment:

  1. Error handling in general
  2. What should happen when a render event is triggered within a render call
  3. Possibly others and I would like to open that discussion to the community
@diffcunha diffcunha linked a pull request Mar 14, 2018 that will close this issue
@toddself
Copy link
Contributor

toddself commented Mar 14, 2018

The way we've resolved this in our app is that all renders are sync -- if we need data from an async source in order to do a render, we either pre-load it into the page when it's emitted from the server (using window.initialState) or we request it when the app is ready and then emit a render once the data is attached to the state, e.g.

app.use(function (state, emitter) {
  state.somethingIneed = false
  emitter.on('get:somethingIneed', () => {
    const data = await getSomethingAsync()
    state.somethingIneed = data
    emit('render')
  })
})
app.route('/', view)

function view (state, emit) {
  if (!state.somethingIneed) {
    emit('get:somethingIneed')
  }
  return html`<div>${state.somethingIneed || ''}</div>`
}

This removes the need for complicated render cycles, and SSR doesn't wait for anything, since we can supply a well-formed state object other ways:

const app = require('express')()
const fe = require('./front-end')
app.get('/', async (req, res) => {
  const data = await getDataAsync()
  const state = {
    somethingIneed: data
  }
  res.send(fe.toString('/', state))
})

@diffcunha
Copy link
Author

I see your point but, in many cases you don't know which data to fetch before render because the route/render-path defines which data will be required. Another typical problem is code splitting, with this async render strategy one could do:

const lazyLoadedRoute = async (state, emit) => {
  const component = await import('./lazyLoadedComponent')
  return h`
    <div>
      ${component(state, emit)}
    </div>
  `
}

There are no complicated render cycles, just straightforward code that works both on server and client.

@diffcunha
Copy link
Author

To make it more interesting, async data loading based on a route param with lazy component:

const lazyLoadedRoute = async (state, emit) => {
  const [ component, data ] = await Promise.all([
    import('./lazyLoadedComponent'),
    getRouteData(state.params.id)
  ])
  return h`
    <div>
      ${component(data)}
    </div>
  `
}

@toddself
Copy link
Contributor

toddself commented Mar 14, 2018

in many cases you don't know which data to fetch before render because the route/render-path defines which data will be required

Both of the examples provided you have the route/render-path before you're asking for the data. The data isn't fetched until the route is being rendered on both the server and client examples. If you don't have the information you need in order to obtain the data you want by that point, there's no where else to get it :).

To make it more interesting, async data loading based on a route param with lazy component:

function someView (state, emit) {
  const componentId = state.params.id
  let showLoader = false
  if (!state.components[componentId]) {
    emit('get:component', componentId)
  }

  if (showLoader) {
    return html`<div class="spinner">Yeah i'm fetching data for you it's cool</div>`
  } else {
    return html`<div>${state.components[componentId]}</div>`
  }
}

As far as lazy-loading routes, the general consensus has been to have that done at the router level, rather than in the component level. There's even a module for it that is under way currently: choo-lazy-route!

There are a couple of solutions to run both network requests simultaneously, including a listener on the navigate event:

app.use(function (state, emitter) {
  emitter.on('navigate', (route) => {
    const id = route.match(/component\/(.+?)$/)[1]
    if (id && !state.components[id]) {
      emitter.emit('get:component', id)
    }
  })
})

Or by overriding the specific click event for the navigation (or likely several other patterns)

@toddself
Copy link
Contributor

I should state I'm not against the concept here -- there are already patterns that exist within choo that prevent us from having to deal with Promises (or async/await) directly in core.

Other @choojs/trainspotters might have different opinions on this however!

@ungoldman
Copy link
Member

For complex components, you might want to consider something like https://github.com/choojs/nanocomponent, which gives more control over conditional rerendering, or halting the update cycle altogether and manually changing DOM elements in cases where that's needed (e.g. third party mapping library).

For asynchronous I/O, in many ecosystems (react/redux, angular, ember, backbone) the convention I've seen is to render an intermediary loading state and fire an event when the async op is finished with a success or failure state, and rerender the component as needed.

my 2 cents :)

@diffcunha
Copy link
Author

Both of the examples provided you have the route/render-path before you're asking for the data.

The last one doesn't, if you look the getRouteData function you'll see that I'm passing a route param (state.params.id), that means that you only know what to fetch once the route is resolved/rendered. So the route/render-path defines which data to fetch. Just for clarity, could you please show me how would you implement this (server and client)?

const view = async (state, emit) => {
  const data = await getRouteData(state.params.id)
  return h`
    <div>
      ${component(data)}
    </div>
  `
}

Regarding your example of lazy-loading routes:

  1. I believe you forgot to change showLoader somewhere
  2. You are storing functions in the state therefore making it non-serialisable
  3. You didn't demonstrate how to make that work in SSR

@diffcunha
Copy link
Author

Thanks for the note @ungoldman. That's has been the way of doing it with React despite all the SSR pain although, the React team is moving to async render very soon: https://www.youtube.com/watch?v=v6iR3Zk4oDY

@toddself
Copy link
Contributor

toddself commented Mar 14, 2018

I believe you forgot to change showLoader somewhere

I did!

if (!state.components[componentId]) {
  emit('get:component', componentId)
  showLoader = true
}

You are storing functions in the state therefore making it non-serialisable

I am not storing functions on the state at all. In the examples above I'm essentially expecting state.components = {}, and either attaching events to the emitter or emitting events. The state should always be serializable.

You didn't demonstrate how to make that work in SSR

That would be up to the server-side http thing you're using, but the idea is essentially the same?

const app = require('express'()
app.get('/component/:id', (req, res) => {
  const data = await getData(req.params.id)
  // same code as the other SSR example
})

Just for clarity, could you please show me how would you implement this (server and client)?

I'm not sure how my example didn't cover that, or can't be easily extrapolated from that? It looks like you're asking that you need fetch very specific data for a given route & other information from that route. You have access to all the route information on the state object in the view, so can make a call that provides both things back.

const view = async (state, emit) => {
  if (!state.data[state.params.id]) {
    emit('get:data', {id: state.params.id, route: state.href})
  }
  
  return h`
    <div>
      ${component(state.data[state.href][state.params.id])}
    </div>
  `
}

One of the things I'm noticing is that your views aren't using data attached to the state object -- this is the "choo-way" do provide data to a component, rather than requesting data specifically for that view. It makes SSR much easier to do since you just inflate a state object and pass it to the toString method, along with the URL you want to trigger; on the server-side your route should be doing nothing other than returning a string populated with the provided data.

As @ungoldman said above, the level of complexity you're looking for is likely better accomplished via nanocomponent which provides some semblance of local state, as well as lifecycle hooks. Choo's default views are designed to be pretty bare-bones and basic (however, that is not set in stone!).

@diffcunha
Copy link
Author

diffcunha commented Mar 15, 2018

@toddself, a couple of notes:

Thanks for fixing showLoader. Unfortunately that code will just render a loading screen on SSR, with no data at all, not very interesting

I understood you were putting components inside state.components, not data. Given that, can you please demonstrate how to do component lazy-loading both on server and client?

That would be up to the server-side http thing you're using, but the idea is essentially the same?

I think it's completely different for 2 reasons:

With your approach you need to declare your routes twice, in Choo router and Express. My approach only uses Choo router:

const server = require('express')
server.get('*', (req, res) => {
  const html = await app.toString(req.url, state)
  // ...
})

With your approach you have to know beforehand which data to fetch for every route, and you have to declare it twice: in the Express handler and somewhere else in your Choo app. With my approach, the rendering path will define which data to fetch and you declare it only once:

const view = async (state, emit) => {
  const data = await getDataForThisView(state.params.id)
  return h`
    <div>${dummyComponent(data)}</div>
  `
}

I'm not sure how my example didn't cover that

I believe your example won't render the data on the server. There is only one render pass on the server so what will happen is something like:

  1. A get:data event is emitted once the render reaches that point
  2. The listener will fetch the data asynchronously
  3. The render continues but no data is in the state
  4. The fetch response arrives and a render event is emitted, it won't have any effect on the server

So the output of it will be an empty view, or a loading screen, a spinner, etc

One of the things I'm noticing is that your views aren't using data attached to the state object

I over simplified the data fetching functions for demo purposes, in fact what happens is more like:

async function getData(state, emit, id) {
  if (state.data[id]) {
    return state.data[id]
  } else {
    const data = await fetchFromAPI(id)
    emit('data:store', id, data) // will make state.data[id] = data
    return data
  }
}

const view = async (state, emit) => {
  const data = await getData(state, emit, state.params.id)
  return h`
    <div>
      ${component(data)}
    </div>
  `
}

@goto-bus-stop
Copy link
Member

I worry that having Promises arbitrarily deeply in the tree will be a major footgun. for example:

async function someElement () {
  var response = await fetch('/some-data')
  var data = await response.json()
  return html`<p>${data.something}</p>`
}

This would do an HTTP request on every rerender and wait for it to complete before morphing.
Imo, allowing top level route handlers to return promises would be fine, but I'm not sold on having async functions way down the tree. If we allow promises from top level routes, you could still use a custom html string tag that handles promises down the tree, but it would be opt in and only for when you pinky promise that you'll use caching for fetch() etc :)

With _experimental_prefetch in bankai, we can already make server side rendering wait for data. choo-lazy-route uses it on the server for code splitting: https://github.com/goto-bus-stop/choo-lazy-route/blob/master/index.js#L44-L46 I think we could also build awareness of _experimental_prefetch into the choo router too.


Imo when a store doesn't fit the use case it's also fine to patch the app in a normal function, like

var app = choo()
require('./async-render')(app)

Alternatively, you could extend Choo, as it is Just Prototypes:

module.exports = class AsyncChoo extends Choo {
  async start () {
  }
  async toString () {
  }
}
// app.js
var app = new AsyncChoo()

Neither of those are ideal since you would have to copy quite a lot of choo code, but it may be helpful to bridge the gap until we land something that can support this.

@toddself
Copy link
Contributor

I still posit you have everything you need at this point to do this without making the view load the data directly:

With your approach you need to declare your routes twice, in Choo router and Express. My approach only uses Choo router:

const server = require('express')
server.get('*', (req, res) => {
  const state = {}
  state[component][id] = await getDataAsync(req.url)
  const html = await app.toString(req.url, state)
  // ...
})

All of the information you need about the request you're processing is there -- query string params, route info, etc. There is nothing at all that makes this pattern not work from the information you've shown me.

I believe your example won't render the data on the server. There is only one render pass on the server so what will happen is something like:

It does (I'm using this literally right now). The server never fires the get:data call because I've pre-populated it on the state object before passing into .toString:

  const data = await getDataAsync()
  const state = {
    somethingIneed: data
  }
  res.send(fe.toString('/', state))
function view (state, emit) {
  if (!state.somethingIneed) {
    emit('get:somethingIneed')
  }
  return html`<div>${state.somethingIneed || ''}</div>`
}

It only tries to get the data if the object doesn't exist on the state. In the original example, that key gets populated before the state object is passed to choo.

@diffcunha
Copy link
Author

diffcunha commented Mar 16, 2018

@goto-bus-stop I understand your concern, but with purely synchronous render one could still do:

function someElement (state, emit) {
  while(true) {}
  return html`<p>you'll never see me</p>`
}

function someElement (state, emit) {
  reallyLongBlockingCall()
  return html`<p>you'll have to wait to see me</p>`
}

There are also many ways to shoot yourself with sync render. IMO is pretty much the same as promises that never resolve or doing expensive async calls every render.

Another aspect is, there's not really a way to only allow top level handlers to return promises, from the moment they do then is async land for whoever is down the function.

Regarding your suggestion, I also thought of that and I decorated the bel html tag on #646 so that one is able to compose the view synchronously though the underlying components are asynchronous.

const body = children => (state, emit) => html`
  <body>
    ${children.map(child => child(state, emit))}
  </body>
` 

const section = (name, child) => (state, emit) => html`
  <section>
    <h1>${name}</h1>
    ${child(state, emit)}
  </section>
`

// the only 'async' one
const item = id => async (state, emit) => {
  // getData doesn't necessarily trigger a fetch, can read from store if data available
  const data = await getData(state, emit, id) 
  return html`
    <div>
      <h3>${data.title}</h3>
      <p>${data.body}</p>
    </div>
  `
}

app.route('/', body([
  section('First section', item(1)),
  section('Second section', item(9)),
]))

About bankai, I did try it and I think it's a great project but unfortunately it didn't meet all my requirements and I felt it wasn't easy to configure or change according to my needs.

Thanks for the tip regarding patching Choo. I was able to patch Choo using a store but I had to lock to version 6.9.0 because of #638 as it changed the way the stores are initialised.

Please take a look at this simple PWA that I've built with Choo using this async approach, it has some interesting features like:

  • HTTP/2 Push
  • Full SSR (you can disable JS)
  • App Shell Model
  • Service Worker with pre-caching

Demo: https://choo-pwa.xyz
Code: https://github.com/nearform/choo-pwa

@s3ththompson
Copy link
Member

s3ththompson commented Mar 16, 2018

Had a conversation with @yoshuawuyts about this last weekend. In general I think that making views asynchronous in order to support remote data fetching is a bit of a red herring. IMO data fetching should be represented by an explicit loading state in the app, thereby keeping rendering an immediate, pure, and idempotent representation of the state object. Initially I was in favor of keeping views synchronous in order to recommend this pattern. (I threw together nanofetcher to help manage data fetching as a separate part of the component lifecycle.)

However, @yoshuawuyts convinced me there were a few benefits of asynchronous rendering:

  • moving to an API which could eventually support lit-html was a worthy endeavor in-and-of-itself.
  • making lazy loading of views easier

Still: I think there's a distinction between making rendering asynchronous (which makes sense for the above reasons) vs. encouraging data fetching and/or initialization to happen as part of the render (which I think should be discouraged in favor of separate component lifecycle events)

Edit: apparently I was missing the simple way of stating all of this:

no side-effects in render phase.

@diffcunha
Copy link
Author

@s3ththompson I appreciate your comment a lot. I share the same view that rendering should be a pure and idempotent representation of the state, might not agree with the immediate.

AFAIK that doesn't necessarily happen with right now in Choo. Correct me if I'm wrong but due to the synchronous nature of nanobus, if an event that mutates the state is emitted on render, the state change will happen synchronously and during the render call.

I also have to say that the biggest benefit for me is in fact the isomorphic lazy loading of views. The data fetching is a big plus but not necessarily the main point. Given that you mentioned nanofetcher, how would it work in the case of SSR? As far as I understood it hooks to DOM mount.

I like Choo for its simplicity and minimalism. I agree that anti-patterns shouldn't be encouraged but given the minimal nature of the framework there will be always space for them. I believe that can only be avoided by building best-practices.

@kjacobson
Copy link

kjacobson commented Aug 2, 2019

It seems to me that this matters too:

CPU - Rendering complex components might block the main loop affecting the overall experience

If you're running a NodeJS server that's handling requests from multiple clients and not using worker threads to render (i.e generate a full HTML document from components/templates), the handling of request B might be completely blocked by the "rendering" of the HTML for request A, and assembling a string representing an entire HTML document is likely the longest callstack of synchronous code a web server ever executes.

Being able to occasionally yield to other waiting request handlers would result in a slightly longer minimum response time (in situations where only one request is incoming at a time) and could result in a slightly longer average response time per request (for a low-traffic app), but would drastically decrease the maximum response time.

There's probably a difficult middle ground here: you wouldn't want to yield after every component is evaluated (say, per link in a navigation menu), but being able to split document compilation/rendering into ~5 stages could be advantageous. One way to do this would be to be able to opt-in to async rendering (by which I mean yielding immediately before or immediately after evaluating a component's resulting HTML) on a per-component basis, allowing application offers to experiment with the right frequency of yields.

You could do this by wrapping async template evaluation in some sort of promiseImmediately function (like a modern setTimeout(fn, 0)):

const promiseImmediately = (fn, errorCallback) => {
    return new Promise((resolve, reject) => {
        setImmediate(() => {
            try {
                resolve(fn());
            }
            catch (err) {
                if (errorCallback) {
                    resolve(errorCallback(err));
                } else {
                    reject(err);
                }
            }
        });
    });
};

where fn is the template evaluation function (h for a given component). This is a bit heavy-handed, but you could fail evaluating any component. Then you could await the evaluation of each nested component.

This is meant to be more food for thought than a choo/nanohtml-ready solution, but I mean to cast a vote for the notion that, in Node, not blocking the main thread with long synchronous "render" operations for servers meant to handle multiple simultaneous threads is kind of a big concern.

I've implemented a naive version of this for non-choo SSR and measured notable performance improvements under heavier loads.

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 a pull request may close this issue.

6 participants