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

v6.x.x / v7.0.0 - wish list / roadmap #563

Closed
yoshuawuyts opened this issue Sep 15, 2017 · 15 comments
Closed

v6.x.x / v7.0.0 - wish list / roadmap #563

yoshuawuyts opened this issue Sep 15, 2017 · 15 comments

Comments

@yoshuawuyts
Copy link
Member

I want to gather a bit of sentiment for what people might be missing / what you feel might improve. The same as in v6, it'd be nice if all that was needed to update the major was npm install --save choo@7, and that's it.

So far my wish list is:

  • Figure out how to make routes render async, so we can apply route-based code splitting when needed. Not necessarily breaking, but it might just turn out that we need to rewrite the router (again, sigh - haha).
  • Remove the unique labels from the tracing API measures, there's no need for them when using a PerformanceObserver like in choo-log.
  • Replace xtend with Object.assign() - https://polyfill.io provides a fallback for browsers that don't support it, so for most people this will be a strict improvement.
  • choo/component, using nanocomponent - Ideally it'd be nice if we could formalize tracing / an event system with this too. That way we keep blazing ahead in terms of debugging.
  • Reading out from a global state (e.g. window.initialState) to provide out of the box "rehydration" of state that was passed on the server.

It's still early, but I'd be interested in hearing input from people on what you find tricky, which things you love, and which things you struggle with! Thanks! ✨

@toddself
Copy link
Contributor

toddself commented Sep 15, 2017

I would love if we could figure out a way to get code-splitting to allow for code-split bundles to gain access to app.use without using the window to store your app instance to it's accessible.

var app = typeof window.app === 'undefined' ? choo() : window.app

@rafaismyname
Copy link

It would be nice to use barracks seamlessly again...
Also, bringing nanocomponent closer will be awesome to build some stateful components :)

@graforlock
Copy link
Member

graforlock commented Sep 15, 2017

Point no. 1 - routes rendered async, you might want to check out universal-router, i think they got it quite right, I also implemented their router in many side projects of mine.

Excerpt:

const router = new UniversalRouter({
  path: '/hello/:username',
  async action({ params }) {
    const resp = await fetch(`/api/users/${params.username}`);
    const user = await resp.json();
    if (user) return `Welcome, ${user.displayName}!`;
  },
});

Of course, this is more verbose with additional property names since choo doesn't use these - its, however, trivial to omit :-)

@YerkoPalma
Copy link
Member

Some clever way to do page transitions/animations. There has been some discussion before in #487 I'm particulary interested in page transitions, because element component specific animations should be resolved particullary in views or nanocomponents IMO

@graforlock
Copy link
Member

graforlock commented Sep 16, 2017

Extending router by a before hook would just do, wouldn't it? Its also non-breaking:

app.route(route, component, beforeHook)

@tornqvist
Copy link
Member

tornqvist commented Sep 20, 2017

Here's a radical thought regarding async routes: if they were generator functions one could progressively render a view as data/resources become available.

Also, I do miss a standard way of handling errors in choo. Wrapping the route handlers with a try/catch and emitting a standard error event upon a route crashing or a yielded Promise rejecting would be super!

var html = require('choo/html')
var choo = require('choo')

var app = choo()
app.use(api)
app.route('/', mainView)
app.mount('body')

function * mainView(state, emit) {
  if (state.error) {
    return html`
      <body>
        <h1>Oops!</h1>
        <p>This happened: ${state.error}</p>
        <button onclick=${onclick}>Try again</button>
      </body>
    `
    function onclick() {
      emit('fetch')
    }
  }

  if (!state.data) {
    yield html`
      <body>
        <h1>Loading</h1>
      </body>
    `
    yield emit('fetch')
  }

  return html`
    <body>
      <h1>${state.data}</h1>
    </body>
  `
}

function api (state, emitter) {
  state.data = null
  
  emitter.on(state.events.ERROR, function (err) {
    state.error = err.message
    emitter.emit(state.events.RENDER) // Override current render cycle
  })

  emitter.on('fetch', async function () {
    var response = await fetch('/api')
    state.data = await response.text()
    state.error = null
    emitter.emit(state.events.RENDER) // Ignored during async render cycle
  })
}

@chrisdwheatley
Copy link

Can a build with assert's removed be published alongside the standard build?

I had some difficulty 'unassertifying' via webpack (got it working in the end) and the assert library is quite large so it'd be nice to have a simpler way to reduce bundle size.

@yoshuawuyts
Copy link
Member Author

@chrisdwheatley nice one! - yeah was thinking we move to https://github.com/emilbayes/nanoassert using the package.json browser field. We've done that for a few modules already, and it seems to work real well 😁

@graforlock
Copy link
Member

I think no1 priority should be the apparent performance issue as of now, thats a rather big issue imo

@niallobrien
Copy link

@graforlock Any particular examples?

@graforlock
Copy link
Member

graforlock commented Oct 5, 2017

Not many real world ones (not in a simple SPA website) but creating, updating and appending thousands of divs obviously keep recreates the full DOM paint and maybe needs some good caching/memoizing mechanism (if possible).

@kareniel
Copy link
Member

I started doing this for async routes, it's nothing fancy or revolutionary but I'm putting it here in case it might come in handy in figuring out a framework-level solution.
I needed it because I'm doing server-side rendering and want to make sure I fetch the data before
I return the initial render.

// client side

app.use(function asyncInitialState () {
  return function (state, emitter, app) {
    const routeFn = app.route.bind(app)
    const onCallback = initialState => Object.assign(state, initialState)

    app._getInitialState = {}
    app.route = function (routeName, fn, getInitialState) {
      routeFn(routeName, fn)
      app._getInitialState[routeName] = getInitialState
    }

    emitter.on(state.events.NAVIGATE, () => {
      const getInitialState = app._getInitialState[state.route]
      if (!getInitialState) return
      getInitialState(onCallback)
    })
  }
})

app.route('/blog', layout(blogPage), async () => {
  const initialState = {
    blog: {
      fetching: false,
      posts: await fetch(endpoint).then(res => res.json())
    }
  }

  return initialState
})
// server side

async function routeHandler (req, res, next) {
    const pathname = url.parse(req.url).pathname
    if (regex.test(pathname)) return next()

    var initialState = {}
    var getInitialState = client._getInitialState[pathname]

    if (getInitialState) initialState = await getInitialState()

    const html = client.toString(pathname, initialState)

    pump(req, res)
    res.end(html)
  }

@brechtcs
Copy link

I think we it's possible to rewrite the render handling in such a way that it can support both sync and async routes without changing the syntax. This is loosely based on the way Gulp deals with the fact that tasks can both be sync and async. (I've removed some nanotiming calls for clarity.)

  this.emitter.prependListener(self._events.RENDER, nanoraf(function () {
    self.state.href = self._createLocation()
    var res = self.router(self.state.href)

    if (res instanceof HTMLElement) {
        var newTree = res
        nanomorph(self._tree, newTree)
    } else if (res && res.then typeof === 'function') {
        res.then(function (newTree) {
            assert(newTree instanceof HTMLElement, '...')
            nanomorph(self._tree, newTree)
        })
    }
}))

This way the current sync routes operate in the same way as they do now, plus you can also pass in a route handler which returns a promise (or just an async function, since they're based on promises under the hood).

It might be possible to support callbacks too, as a third argument for route handlers, i.e. (state, emitter, done). If we added the required support to nanorouter, that would change the above to:

  this.emitter.prependListener(self._events.RENDER, nanoraf(function () {
    self.state.href = self._createLocation()
    var res = self.router(self.state.href, function (err, newTree) {
         if (err) return //not exactly sure how to handle this yet, feedback welcome :p
         assert(newTree instanceof HTMLElement, '...')
         nanomorph(self._tree, newTree)
    })

    if (res instanceof HTMLElement) {
        var newTree = res
        nanomorph(self._tree, newTree)
    } else if (res && res.then typeof === 'function') {
        res.then(function (newTree) {     
            assert(newTree instanceof HTMLElement, '...')
            nanomorph(self._tree, newTree)
        })
    } else {
        //do nothing, should be handled by the callback above
        //maybe some kind of assertion could be here added to be sure
    }
}))

@brechtcs
Copy link

brechtcs commented Nov 24, 2017

I've experimented a bit with async routes on my own fork, and it seems the biggest challenge would be to keep supporting the app.toString() functionality, which should probably always stay synchronous.

Maybe an optional async resolve mechanism would be more appropriate. A possible API could be:

app.route('/', mainView, {resolve: fetchThings})

function mainView (state, emit) {
    return html`<body>
        <ul>${state.things.map(listItem)}</ul>
    </body>`
}

function fetchThings (state, done) {
    api.get('/things', function (err, res) {
        if (err) {
            return done(err)
        }
        state.things = res.data
        done()
    }
}

bates64 added a commit to bates64/nanochoo that referenced this issue Jan 5, 2018
As described by choojs#563:

> https://polyfill.io provides a fallback for browsers that don't support it, so for most people this will be a strict improvement.
@tornqvist tornqvist mentioned this issue Jun 8, 2019
6 tasks
tornqvist pushed a commit that referenced this issue Jun 11, 2019
As described by #563:

> https://polyfill.io provides a fallback for browsers that don't support it, so for most people this will be a strict improvement.
@tornqvist
Copy link
Member

I'm closing this since v7.0.0 has been published and the discussion on async render has been picked up elsewhere: #653 #646 #645

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

No branches or pull requests

10 participants