feature idea: a cleaner start() function

[yoshuawuyts]

Right now we've got this weird behavior in start() that the API to append to something rendered on the server is different when running it from root. Internally this means there's a whole bunch of if statements that make the code look less than ideal:

  function start (selector, startOpts) {
    if (!startOpts && typeof selector !== 'string') {
      startOpts = selector
      selector = null
    }
    startOpts = startOpts || {}

    _store.model(appInit(startOpts))
    const createSend = _store.start(startOpts)
    _router = start._router = createRouter(_defaultRoute, _routes, createSend)
    const state = _store.state({state: {}})

    if (!selector) {
      const tree = _router(state.location.pathname, state)
      _rootNode = tree
      return tree
    } else {
      onReady(function onReady () {
        const oldTree = document.querySelector(selector)
        assert.ok(oldTree, 'could not query selector: ' + selector)
        const newTree = _router(state.location.pathname, state)
        _rootNode = yo.update(oldTree, newTree)
      })
    }

And in addition the API can be two things:

// basic
const choo = require('choo')
const app = choo()

const tree = app.start()
document.body.appendChild(tree)

// server rendering
const choo = require('choo')
const app = choo()

if (require.parent) {
  module.exports = app
} else {
  app.start('#my-root-node')
}

---

Now I'm not a fan of this. I'm glad we put it in there to make it at least possible to render on the server, but looking back I think we can create a better API. I'm not a fan of passing in arguments that change the return type, which in turn also must be wrapped in order to export. The upgrade doesn't feel intuitive. It's too many things changing in different directions I feel. With eyes on the upgrade curve (or smooth learning curve, w/e) I reckon this might be a nicer solution:

// basic
const choo = require('choo')
const app = choo()

const tree = app.start()
document.body.appendChild(tree)

// server rendering
const mount = require('choo/mount')
const choo = require('choo')

const app = choo()

if (module.parent) {
  module.exports = app
} else {
  const tree = choo.start()
  mount('#my-root-node', tree)
}

The differences with this might not seem like a lot, but it plays further into the narrative of "removing magic" and only using native DOM methods. I think it would make the code clearer and better communicate intent (answering the important "why" for why the API is the way it is). Internally it also helps the implementation by not bundling bytes unless absolutely necessary:

function start (selector, startOpts) {
  startOpts = startOpts || {}

  _store.model(appInit(startOpts))
  const createSend = _store.start(startOpts)
  _router = start._router = createRouter(_defaultRoute, _routes, createSend)
  const state = _store.state({state: {}})

  const tree = _router(state.location.pathname, state)
  _rootNode = tree
  return tree
}

// mount.js
const onReady = require('on-ready')
const assert = require('assert')
const yo = require('yo-yo')

module.exports = mount

function mount (selector, tree) {
  assert.equal(typeof selector, 'string')
  assert.equal(typeof tree, 'object')

  onReady(function onReady () {
    const oldTree = document.querySelector(selector)
    assert.ok(oldTree, 'could not query selector: ' + selector)
    _rootNode = yo.update(oldTree, tree)
  })
}

Because we're changing the API this would definitely be a breaking change. But I wanted to hear your opinions on this - what do you all think?

[timwis]

👍 to separating out server functionality. I haven't used it (and don't anticipate needing to) so I don't really know why there's separate mounting logic (why doesn't user just use dom APIs like we do in client). Also it looks like _rootNode would be undefined above.

On a separate note, there are some app options that have to be passed to choo(...) and some that are passed to start(...). I mix them up sometimes. Would it be worth having them all go in the same place? (Probably separate issue but worth considering if doing breaking change to start anyway)

[yoshuawuyts]

On a separate note, there are some app options that have to be passed to choo(...) and some that are passed to start(...)

#192 would lay out the groundwork for this; in a major version we could deprecate all options passed to choo(), only using .start(opts). Would that be an adequate resolution?

[yoshuawuyts]

Also it looks like _rootNode would be undefined above

Hm, I think if we could assert if the new node is different from the old node, then a bug a mismatched has happened / a node wasn't morphed but replaced instead. Think that should be reasonable

[yoshuawuyts]

closing as it's staged on v4 already