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

Introduce pre-render resolve event #603

Closed
wants to merge 2 commits into from

Conversation

brechtcs
Copy link

Since my experiments with async routing on this branch have been a bit of a struggle so far, this is a potential alternative. This approach starts from the idea that the proper way to do async stuff within the current architecture is through nanobus, and that the routing logic is best kept fully synchronous.

This implementation checks if the developer has registered any resolve listeners in the application. If yes, that/those listeners are triggered instead of render, and the developer is responsible for triggering render from there. If no resolve listeners are registered, the current behaviour for render is kept.

An example of an asynchronously rendered route using this API would be:

app.use(function (state, bus) {
    bus.on('resolve', function() {
        if (state.route !== '/async') {
            return bus.emit('render')
        }
        http.get('https://api.example.com' + state.route, function (err, data) {
            if (err) return handle(err)
            state.data = data
            bus.emit('render')
        })
    })
})

// standard synchronous routes:
app.route('/', home)
app.route('/sync', viewSyncData)
// route that asynchronously displays `state.data`:
app.route('/async', viewAsyncData)

This implementation is already much more stable than my experiment on the other branch, but I'm not sure if the API would work for everyone. Feedback welcome!

index.js Outdated
self.emitter.emit(self._events.RENDER)
setTimeout(scrollToAnchor.bind(null, window.location.hash), 0)
var resList = self.emitter._listeners[self._events.RESOLVE]
var resCount = resList ? resList.length : 0;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with this implementation, it might be cleaner to move this logic to a countListeners or hasListener method in nanobus itself. But yeah, we'll cross that bridge if we get there.

@brechtcs brechtcs mentioned this pull request Nov 30, 2017
@lemmon
Copy link

lemmon commented Dec 1, 2017

How is this new functionality different from just doing this:

function mainView (state, emit) {
  const { data } = state
  
  if (!data) {
    fetch('http://www.example.com/data.json').then(() => {
      state.data = 1
      emit('render')
    })
    return html`
      <div class="loader">Loading...</div>
    `
  }
    
  return html`
    <div class="content">
      Form is loaded!
    </div>
  `
}

@timwis
Copy link
Member

timwis commented Dec 4, 2017

@lemmon if render is triggered while the fetch is in progress (say, from clicking a button on the UI), it will trigger another fetch.

@brechtpm I like this idea but am wondering why defer the render as opposed to initially rendering without data (and handling that condition in the UI)? Simplifies it a bit (i think).

@yoshuawuyts
Copy link
Member

@brechtpm hey, yeah thanks for this PR. I think this is a pretty cool approach, but we should wait for components to land first. The reason why I'm saying that, is because with components we'll probably want to trigger data fetch events from the components themselves - which means that if we defer rendering, those events won't get started.

Feel this is very close to what we need tho; thanks so much for doing all the work with experimentation on this! (oh, and sorry for not replying, was on vacation for the first half of December, and then forgot about this PR haha.)

@brechtcs
Copy link
Author

@yoshuawuyts Cool, haven't really been following the developments on components recently, but I'll keep an eye on it from now.

@timwis I think the use case that you mention, where you want to do an initial render without data, is already covered by the current setup. I think you'd only really need this resolve event in cases where you explicitly don't want to do the initial render.

To give a specific example, the use case I was looking to solve is the one where only part of your application is a client-side choo app, with the other part being fully server-rendered. I've been experimenting with this resolve event to fetch these server-rendered views and then pass these through morphdom to render them as if they were part of the choo app. This makes it basically a morphdom-using version of turbolinks, with the added advantage that parts of your app can still be kept fully client-side using choo.

I've already created a basic implementation of this idea here. (This a fully static site, rather than an app, but the idea is the same.) Planning to release it as a standalone choo plugin once this PR (or a functionally similar async resolve mechanism) lands.

@s3ththompson
Copy link
Member

@brechtpm I just saw this PR! I've been thinking about the same issue, but went about solving it in a slightly different way.

I agree with @yoshuawuyts in that I think it's slightly cleaner if data fetching is localized to components. That way, different components can fetch async data in different ways--even from different APIs if necessary.

I put together a little library called nanofetcher that extends nanocomponent and works with choo out-of-the-box.

The default behavior of a nanofetcher component is to render a loading placeholder after a route synchronous resolves and then fetch async data and hydrate the component. However, nanofetcher components also expose a .prefetch method, allowing you to fetch data asynchronously before the component is first rendered (and before you navigate to a route, if desired). (Nanofetcher also deals with all the messy stuff like making sure multiple renders don't trigger multiple fetches.)

This means you can do something like the following, to achieve your original goal of fetching data asynchronously before the new route renders.

var Nanofetch = require('nanofetcher')
var choo = require('choo')
var html = require('choo/html')
var http = require('http')

// This is mostly just nanofetcher / nanocomponent boilerplate
function AsyncPage () {
  if (!(this instanceof AsyncPage)) return new AsyncPage()
  Nanofetch.call(this)
}

AsyncPage.prototype = Object.create(Nanofetch.prototype)
AsyncPage.prototype.constructor = AsyncPage

AsyncPage.identity = function (route) {
  return route
}

AsyncPage.prototype.init = function (route) {
  this.route = route
}

AsyncPage.prototype.update = function (route) {
  return route !== this.route
}

AsyncPage.prototype.placeholder = function () {
  return html`<div>Loading...</div>`
}

AsyncPage.prototype.hydrate = function (data) {
  return html`<div>${data}</div>`
}

AsyncPage.prototype.fetch = function (cb) {
  // Here's where you specify what remote data to fetch
  http.get(`https://api.example.com/${this.route}`, (err, data) => {
    if (err) return cb(err)
    cb(null, data)
  })
}

var asyncPage = new AsyncPage()

var app = choo()

app.route('/', (state, emit) => {
  return html`<body>
    <a href="/example" onclick=${preload}>Example Async Page</a>
  </body>`

  // by calling `preventDefault` here, we hijack Choo's normal handling of links
  // in order to first prefetch the data for the next route and then we just
  // manually tell Choo to navigate when the prefetching is done
  function preload(e) {
    e.preventDefault()
    asyncPage.prefetch('example', (err) => {
      if (err) return emit('error', err)
      emit(state.events.PUSHSTATE, '/example')
    })
  }
})
app.route('/:route', (state, emit) => {
  // this will immediately render the hydrated data since we already
  // prefetched it, but still shows a loading placeholder if someone
  // visits this link directly (e.g. a hard refresh)
  return html`<body>
    ${asyncPage.render(state.params.route)}
  </body>`
})
app.mount('body')

I've been meaning to put together a little example app with more sophisticated handling of this prefetching stuff + component instance caching--hope to get to it soon...

@brechtcs brechtcs closed this Dec 24, 2018
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 this pull request may close these issues.

5 participants