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

Discussion: choo/component #593

Closed
yoshuawuyts opened this issue Oct 27, 2017 · 76 comments
Closed

Discussion: choo/component #593

yoshuawuyts opened this issue Oct 27, 2017 · 76 comments

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Oct 27, 2017

update (03/11/17)

The initial design in this post isn't great; here's a revised version with support for async loading.


components

When building out applications, components are a great way to slice UI and its
corresponding logic into crisp slices.

We've created nanocomponent as a solution to allow for encapsulation with DOM
diffing. Unforunately it seems to be a tad verbose, which means people have
gone around and built wrappers to make things a little simpler.

These wrappers seem to be mostly focused on two things:

  1. Manage component instances.
  2. Remove boilerplate involved with create a component.

Challenges

  • Because of the modular nature of components, we can't do the centralized
    tangly thing that other frameworks can do. Unfortunately :(.
  • We also took it upon ourselves to use "The Web Platform". This means we're
    limited by odd behavior from the DOM. Not great, but it's a tradeoff we've
    taken.
  • Most of what we do is about explicit behavior — explicit re-renders, explicit
    updates, explicit mutations. Being overly explicit can result in boilerplate,
    and repetition — the art is to find balance.

Example

A basic component looks kinda like this:

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

module.exports = class Button extends component {
  createElement () {
    return html`
      <button>
        Click me
      </button>
    `
  }

  update () {
    return false
  }
}

But that's not super debuggable. Ideally we would allow devtools to inspect all
of the state, events, and timings.

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

module.exports = class Button extends component {
  constructor (name, state, emit) {
    this.state = state
    this.emit = emit
    super(name)
  }

  createElement () {
    return html`
      <button onclick=${this._onclick}>
        Click me
      </button>
    `
  }

  update () {
    return false
  }

  _onclick (e) {
    this.emit('increment', 1)
  }
}

Ok cool, now we can emit events on the global emitter. When the component is
constructed it just needs to be passed the elements. This can be done with
something like shared-component. It requires a bit of boilerplate — the exact
implementation is not the point.

Instead I was thinking it might be neat if we could do component registration
in a central spot — as a new method on an instance of Choo.

var choo = require('choo')

var app = choo()
app.component('button', require('./components/button'))
app.route('/', require('./views/main'))
app.mount('body')

Yay, so now we have component registration going on. Folks can proceed to slice
up state in whatever way they want. To render components, I was thinking we'd
add another method to views so components become accessible.

var html = require('choo/html')

module.exports = function view (state, emit, components) {
  return html`
    <body>
      ${components.button.render()}
    </body>
  `
}

components here is an object, similar in behavior to shared-component. I
considered stealing some of component-box's behavior, and abstract away
the .render() method — but when a component requires another component, it
wouldn't have any of those abstractions, and we'd introduce two different APIs
to do the same thing. I don't think that might be a good idea. Or maybe we
should do it? Argh; this is a tricky decision either way (e.g. consistency +
learning curve vs ease of use, half the time).

Oh, worth noting that because components can now be registered with Choo, we
should probably extend Nanocomponent to not require the whole boilerplate:

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

module.exports = class Button extends component {
  createElement () {
    return html`
      <button onclick=${this._onclick}>
        Click me
      </button>
    `
  }

  update () {
    return false
  }

  _onclick (e) {
    this.emit('increment', 1)
  }
}

Ok, that was cool. Exciting!

Yeah, I know — I'm pretty stoked about this too. Now it is worth pointing out
that there's things we're not doing. Probably the main thing we're not doing is
managing lists of components. Most of this exists to smooth the gap between
application-level components, and the views itself. Our goal has always been to
allow people to have a good time building big app — whether it's getting
started, debugging or tuning performance.

Now there's some stuff we'll have to do:

  • We're showing the class keyword everywhere. I think it's useful when
    building applications because it's way less typing than alternatives. Bankai
    would need to support it.
  • We'd need to teach people how to use (vector) clocks to keep track of
    updates. I don't like shallow compares, and just so much boo — let's have
    some proper docs on the matter.
  • We need to think about our file size marketing. You can totes write a whole
    app without any components, and like we should promote it — it's heaps easy.
    But with components, certain patterns become kinda nice too. Is it part of
    choo core? Wellll, kinda — not core core — but core still.

What doesn't this do?

Welllllll, we're not doing the thing where you can keep lots of instances of
the same component around. I think dedicated modules for that make sense —
Bret's component array thing is probably a good start, and work for more
specialized cases from there. For example having an infinite scroller would be
neat.

When will any of this land?

Errrrr, hold on there — it's just a proposal for now. I'm keen to hear what y'all think! Does
this work? Where doesn't this work? Do you have any ideas of how we should do a
better job at this? Please share your thoughts! :D

Summary

Wellllllp, sorry for being super incoherent in this post; I hope it sorta makes sense. The Tl;Dr is:
add a new method for Choo called .component, which looks like this:

index.js

var choo = require('choo')

var app = choo()
app.component('button', require('./components/button'))
app.route('/', require('./views/main'))
app.mount('body')

components/button.js

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

module.exports = class Button extends component {
  createElement () {
    return html`
      <button onclick=${this._onclick}>
        Click me
      </button>
    `
  }

  update () {
    return false
  }

  _onclick (e) {
    this.emit('increment', 1)
  }
}

views/main.js

var html = require('choo/html')

module.exports = function view (state, emit, components) {
  return html`
    <body>
      ${components.button.render()}
    </body>
  `
}

cc/ @bcomnes @jongacnik @tornqvist

@yoshuawuyts
Copy link
Member Author

From IRC

bret> one other weird little domain problem is how to handle external events that want to trigger one-off method calls on a component
01:51 <bret> for example say you want to scroll a component to a particular position
01:51 <bret> the functional/obvious answer is store scroll state in an app store
01:51 <bret> but there are issues wit that, what happens if you re-render when scrolling
01:51 <bret> fucks with the internal state of browser scrolling
01:52 <bret> im not making any sense
01:52 <bret> sorry its been a long day/week
01:52 <yoshuawuyts> bret: you're making perfect sense to me
01:52 <blahah> bret: agree, makes perfect sense
01:52 <bret> sometimes you just want to send a one-off signal to a component
01:52 <bret> rather than fully manage state
01:53 <blahah> i generally do such things with a plugin in app.use that controls the state for one component
01:53 <bret> perhaps thats a deal with state-devil but it lets you avoid debouncing io input
01:53 <yoshuawuyts> bret: perhaps we could do `app.component('name', myComponent, arg1, arg2, arg3)`
01:53 <blahah> then you can message to it from the plugin from another component
01:53 <yoshuawuyts> bret: arg1 could be a dedicated emitter, or some other hack to listen to specific events
01:53 <blahah> and I have component-scoped state
01:54 <bret> yoshuawuyts: yeah I thought about that... easily abusable but you could somehow expose emitter in components
01:54 <bret> or perhaps components could expose a store to be registered
01:54 <bret> blahah: would love to see an example of tat
01:54 <bret> that*
01:54 <yoshuawuyts> bret: yeah, I'm feeling the former would probably end up being a major footgun
01:55 <blahah> wait is there a new api for components?
01:55 <yoshuawuyts> bret: the latter might make a lot of sense
01:55 <blahah> maybe i'm out of date
01:55 <yoshuawuyts> blahah: https://github.com/choojs/choo/issues/593
01:55 <bret> blahah: no, see that proposal
01:56 <bret> yoshuawuyts blahah https://github.com/hypermodules/hyperamp/blob/master/renderer/stores/player.js#L54
01:56 <bret> thats a choo store importing an instance of a component, and calling methods on it based on events
01:57 <yoshuawuyts> bret: yeah, thinking `exports.component` + `exports.store` could be picked up from `.component()`
01:57 <bret> I know its more reliable to pure-render on state, but sometimes you can avoid a whole lot of weirdness by just calling a method
01:57 <bret> yoshuawuyts: interesting
01:58 <bret> im going to print this out
01:58 <bret> and read it with more attention tonight
01:58 <blahah> I don't see why you can't just call emitter.emit('someothercomponent:dosomething') in that component
01:59 <bret> components dont have a built in way to listen for events
01:59 <blahah> maybe I'm misunderstanding the whole problem
01:59 <yoshuawuyts> bret: thinking the main constraint for components would be that they can't register listeners; doing a store + component export would allow for more flexibility in that regard; while not really breaking the separation of stuff
02:00 <bret> say you want to have one component trigger a stateless action on another component
02:00 <yoshuawuyts> blahah: haha, no feel like you're following along alright C:
02:00 <blahah> oh ok, i use temporary state markers
02:01 <bret> blahah: the issue with temporary state markers is that you have to set and clear them, and they may interrupt browser behavior like scrolling
02:01 <blahah> component A emits 'componentb:dothis', the plugin controlling state for B sets a flag 'doingsomething', B does it and emits 'somethingdone'
02:01 <blahah> yeah that's why I made choo-asyncify
02:03 <blahah> you need it to happen sync?
02:03 <bret> no
02:03 <bret> async is fine
02:11 <bret> @yoshuawuyts when you call `components.button.render()` in a view
02:11 <bret> when does it create a new component?
02:11 <bret> does it just re-use existing instances in the order they are called?
02:13 <yoshuawuyts> bret: oh dang, yeah so there's a problem with this — it doesn't work with code splitting in the slightest
02:13 <yoshuawuyts> bueh
02:13 <yoshuawuyts> bret: the idea was to share instances; create it the first time it's called
02:13 <yoshuawuyts> bret: e.g. so you can share an instance of `footer` or something in between all pages
02:15 <bret> im totally behind in code splitting
02:15 <yoshuawuyts> bret: haha, yeah I guess it doesn't matter at all for Electron :P

@bcomnes
Copy link
Collaborator

bcomnes commented Oct 28, 2017

Also:

@ghost
Copy link

ghost commented Oct 29, 2017

How will this affect public packages that try to be "components" which are not part of an app? I think the way they do that in react is not great where the components you use must agree on the version of react you're using. Instead of passing down the implementation of react (dependency injection) they opt for either not depending on react or updating semver all the time and possibly getting incompatible versions. Each of those seems like a bad way to do it.

@ghost
Copy link

ghost commented Oct 29, 2017

What do people think about a convention for components, particularly published ones, where the signature is:

module.exports = function (component) {
  return class extends component { ... }
}

Then when you app.component() it passes in the component class as the first arg automatically. That would avoid some of the versioning problems using a lightweight dependency injection mechanism. You could also do this with choo/html but I think that is less of a problem. But if you wanted to, you could do something like:

module.exports = function (component, html) {
  return class extends component { ... }
}

This way you wouldn't have to put choo in your package.json at all if you publish a choo component. And people could publish libs that shim choo components out for other frameworks for maximum reuse.

@jongacnik
Copy link
Member

Interesting! Extending components with choo's state/emit like this might be very convenient and a cool way to build some conventions.

An initial wonder 🤔 I have is giving components a front-and-center stage like this begins to conceptually tie them a tad closer to choo. Something rad about nanocomponent right now is actually it's lack of relationship to choo. It works super with choo, but because it doesn't have access to things like state or emit, you've got no choice but to think about your components as totally separate, modular things.

I know nothing proposed here changes nanocomponent itself or prevents people from writing modular nanocomponents, but a subtle implication may be that peoples (esp new-to-choo-ers) start thinking about their components as choo-components™, rather than just components.

Not sure if that makes sense, but basically, I think the fact that nanocomponent is separate is a possible asset. Thinking here especially about wanting to encourage an economy of frameork-less components (nanocomponent-adapters), and about encouraging people to write stuff which doesn't create lock-ins (philosophy).

I think passing in handlers as props isn't really too bad? Still debuggable, still concise, and overall maybe a bit more modular?

components/button.js

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

module.exports = class Button extends Component {
  createElement (handleClick) {
    return html`
      <button onclick=${handleClick}>
        Click me
      </button>
    `
  }

  update () {
    return false
  }
}

view.js

var html = require('choo/html')
var Button = require('./components/button')

var button = new Button()

function view (state, emit) {
  return html`
    <body>
      ${button.render(function (e) {
        emit('increment', 1)
      })}
    </body>
  `
}

All this said, I also understand the angle of maybe wanting to couple components to choo a little bit closer, so I'm very curious to hear what others are thinking!

Other thoughts

  • I basically did this same thing for rooch a little bit back, and there's no doubt it makes things convenient extend preact component prototype choop#10
  • I do think it could be cool to simply proxy nanocomponent in the same way bel is proxied. I think there is something to be said for having access to components out-the-gate, and helps feature-complete choo relative to other options (like react)

@tornqvist
Copy link
Member

I agree with @jongacnik. Tying nanocomponent too tightly to choo could have a negative impact and give the impression of lock in (though it necessarily doesn't have to be so). I'm also not very fond of the idea of having to come up with non-conflicting names of application components and then having to pass them down as references from the top view (not all components are used at the top level, after all).

How about we borrow a page from react-redux and introduce a mechanism to (optionally) extend a component with the global emitter and a way of plucking out the parts of the state that a component is concerned with? I'm not 100% on the feasibility of this syntax but I hope you get the gist of it.

const connect = require('choo/connect')
const Component = require('choo/component')

class Button extends Component {
  constructor (...args) {
    super(...args)
    this.emitter.on('increment', () => this.rerender())
  }

  onclick = () => {
    this.emitter.emit('increment')
  }

  update () {
    return false
  }

  createElement () {
    return html`
      <button onclick=${this.onclick}>
        Clicked ${this.state.count} times
      </button>
    `
  }
}

module.exports = connect(Button, state => ({
  count: state.clicks // `state.clicks` would be handled in some `app.use` fn
}))

Also, you could publish a component w/o the connect part and leave it up to the user how to integrate the component with their app.

@timwis
Copy link
Member

timwis commented Oct 29, 2017

Exciting conversation! Since you're considering passing a components object to views, I'll throw this out there. May be hairbrained and I forget if I've brought it up before, but by adding 4 lines to bel, you can do this:

const html = require('bel')

window.CustomButton = function (props) {
  console.log('Called CustomButton', props)
  return html`
    <button style="background-color: ${props.color}">
      ${props.label}
    </button>
  `
}

const foo = { bar: 1 } // demonstrates object can be passed too
const tree = html`
  <div>
    <CustomButton color="red" label="Add" foo=${foo} />
  </div>
`

document.body.appendChild(tree)

(Just a proof of concept and of course attaching it to window isn't ideal but there are other options, particularly when we have access to the global choo/html)

Separately, my gut tells me we need an easy way to use stateful components in a loop, but I've had a hard time articulating it. Should have written down a test case when I last experienced it.

@jongacnik
Copy link
Member

jongacnik commented Oct 30, 2017

I totally agree with @timwis that being able to “just-works” loop over components is definitely worth a look, though curious what the implications might be? If I’m thinking correctly, seems like to get that to work, choo/nanomorph need to take care of creating/managing instances? And for choo/nanomorph to do that, there'd probably need to be some sort of registry (kinda like proposed!). @yoshuawuyts I know you mention that atm this proposal isn't concerned with managing instances, but maybe its adjacently a first step?

fwiw I feel like with a solid nanomorph algorithm (no leaking proxies) you can keep components as a module and keep instance management as a module and be in really good shape.

I’ve done two builds now using nanocomponent/component-box like this and it’s been rad. Only hiccup was leaking proxies. I would say I even prefer handling instances at the module level to be more granular. Like, with component-box because I provide the instance keys I can be sure an exact component instance is reused exactly when I want it (especially across views). Whereas if this was happening under the hood (like react/preact) sometimes you’ll get a new instance when you don’t want it.

Also, @timwis that syntax for custom els/components is really interesting!

@jongacnik
Copy link
Member

jongacnik commented Oct 30, 2017

@tornqvist Would following a similar pattern to choojs/choop#10 kinda accomplish this? Like the following (untested) code:

choo/component.js

function Component () {}
Component.prototype = Object.create(require('nanocomponent').prototype)
module.exports = Component

app.js

// now you can use a plugin to optionally extend choo/component with emit
app.use(function (state, emitter) {
  require('choo/component').prototype.emit = function (eventName, data) {
    emitter.emit(eventName, data)
  }
})

So now when creating components based on choo/component they'll have access to this.emit, like @yoshuawuyts's components/button.js example above.

Maybe something like this is a pattern for getting emit/state into components, w/o needing to introduce a new method like .component() or needing to pass a components object into views? Though I guess it does assume bringing nanocomponent as a proxy into core ¯_(ツ)_/¯

@YerkoPalma
Copy link
Member

I like what @timwis propose, in fact I tried to start some discussion about it in bel and someone posted rbel. Not sure if is the best way, but I would go something like.

  • A register function in choo, just like what @yoshuawuyts said.
  • A choo/component library in choo
  • Some kind of custom way in bel to render components, trying to avoid functions.

That would be my ideal scenario

@tornqvist
Copy link
Member

tornqvist commented Oct 30, 2017

@jongacnik Something along those lines is what I had in mind. It would fix having to pass down the emitter which messes with any kind of shallow argument diff since emit is a newly declared function on every render.

@timwis I have been down that line of thought many times but always have to stop when I realize that pelo make it impossible, but I think it's a reasonable tradeoff. I guess we're better off laying low in wait for a wide support for Custom Elements, when that'll (kinda) work out of the box.

I also agree with @jongacnik about managing component instances in core. It would be a good way of reaching a community consensus and not have a myriad of modules doing the same thing in their own, little special, way. It's actually the main reason I wrote fun-component. Not so much for the functional syntax but so that I wouldn't have to worry about instances. I wanted to simulate the ease by which one uses components in React, or at the very least, contain the instance identify/create part to the where the component is declared, and not where it is implemented.

@tornqvist
Copy link
Member

tornqvist commented Oct 30, 2017

Btw, nanomorph wouldn't really have to keep track of the instances, the component could do that itself. Whenever render is called it checks the identity method which returns a key by which it looks up the instance in an internal cache. If no component by that key exists it'd create one. Ofc, you'd need some cleanup mechanism to discard old instances. Prob this is more of a nanocomponent feature than a choo/component thing. Just wanted to get it out there.

class Tweet extends Component {
  identity (tweet) {
    return tweet.id
  }
  update () {
    return false
  }
  createElement (tweet) {
    return html`<a href="${tweet.href}">${tweet.text}</a>`
  }
}

@bcomnes
Copy link
Collaborator

bcomnes commented Oct 30, 2017

Whenever render is called it checks the identity method which returns a key by which it looks up the instance in an internal cache. If no component by that key exists it'd create one. Ofc, you'd need some cleanup mechanism to discard old instances. Prob this is more of a nanocomponent feature than a choo/component thing. Just wanted to get it out there.

Thats a fantastic idea.

@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Oct 30, 2017

Btw, nanomorph wouldn't really have to keep track of the instances, the component could do that itself

But how do you access a component before then? E.g. we need something to access the instance from the object, and that value can't live inside the object itself. Or am I missing something?


I'm working on a quick implementation of the stuff ^, available as a standalone module. Usage will look like:

var app = choo()
app.use(require('choo-component-preview')())
app.use(require('choo-devtools')())
app.route('/', function (state, emit) {
  return html`
    <body>
      Hello planet
    </body>
  `
})
app.mount('body')

Mostly for prototyping before we start actually developing a thing. Thanks!

edit: it's late, but wanted to mention that I've been reading all comments. I agree it'd be ideal to not make the API diverge between nanocomponent and choo/component. Good point!

@tornqvist
Copy link
Member

@yoshuawuyts I assume you mean child instances would have to be able to access properties assigned to the base class, right? Well, don't judge me on this, it's probably the most horrific implementation ever and it's really pushing the limits of my prototype-jitsu but the idea is that child instances would have the base instance in their prototype chain. I ran in to some problems with the load handlers being bound twice and the constructors of class require being called with the new keyword (hence the baked in class).

http://requirebin.com/?gist=e3525c507bdca993561dadf9fe22d211

@yoshuawuyts
Copy link
Member Author

@tornqvist oh man, that is scary — I'd be a lil cautious with such an approach because I'd imagine debugging it to be so hard 😅

@tornqvist
Copy link
Member

tornqvist commented Oct 31, 2017

lol, you won't get any objections from me, writing that abomination gave me the chills. I'd love to see a more solid approach but maybe this kind of thing is better done with some wrapper module, independent of choo.

module.exports = spawn(Tweet, function identity (tweet) {
  return tweet.id
})

@jongacnik
Copy link
Member

jongacnik commented Nov 2, 2017

Put together an experimental choo-component plugin based on component-box which implements a bit of the proposed api.

→ little glitch demo (code)

Usage

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

class MyComponent extends Nanocomponent {
  createElement (text) {
    return html`<div>${text}</div>`
  }
}

var app = choo()
app.use(require('choo-component'))
app.component('mycomponent', MyComponent)
app.route('/', mainView)
app.mount('body')

function mainView (state, emit) {
  return html`
    <body>
      ${state.component('mycomponent').render('hello!')}
    </body>
  `
}

Simply introduces an app.component() registry function, and a state.component() component getter function. Because this proof-of-concept runs as a plugin, components are accessed via state for the time being rather than as a third argument to views.

The api is consistent with component-box where a component is accessed by passing the component key as first argument to the state.component function. What's interesting with this is you get multi-instance support/managment out of the box by passing unique keys as second argument:

html`
  <body>
    ${state.component('mycomponent', 'a').render('hello!')}
    ${state.component('mycomponent', 'b').render('hey!')}
    ${state.component('mycomponent', 'c').render('hi!')}
  </body>
`

Notes

  • api v experimental and makes lots of assumptions! just wanted to get something functioning to play around with.
  • it'd be easy to adjust the api a bit too so that it is closer to @yoshuawuyts original proposal. Something like: state.component.mycomponent().render()
  • currently does not extend components with choo state/emit, still not 100% about that pattern
  • You can install the plugin and play around with it using the gist
npm i gist:aae2225406a153211f9898bac02cb329 -S

@tornqvist
Copy link
Member

tornqvist commented Nov 3, 2017

I'm sorry but I don't quite see the benefit of referencing components by keys in an object over path on disk. I have three main concerns with this approach:

  • Keeping track of under which key an instance was stored across the application could become a hassle (especially when working in a team). +1 for solving the caching of instances, though.
  • In my last project I have ~30 components, would I register all of them with the application? I imagine my application entry file looking like a total train wreck.
  • How would one load and register components async?

In the spirit of exploration I gave the identity thing another stab and think we could achieve quite a nice developer experience using static methods on the component constructor. Here reusing the Tweet example:

class Tweet extends Component {
  static identity (tweet) {
    return tweet.id
  }
  update () {
    return false
  }
  createElement (tweet) {
    return html`<li><a href="${tweet.href}">${tweet.text}</a></li>`
  }
}

document.body.appendChild(html`
  <ul>
    ${tweets.map(props => Tweet.render(props))}
  </ul>
`)

The static method render on the constructor would handle instances but one can still create instances "manually". Also, this has the added benefit of creating a global cache for all components and you wouldn't have to know by which key a component was cached.

Component.cache[Tweet.identity({id: 'abc123'})] // => Instance of Tweet with id 'abc123'

A proof of concept: http://requirebin.com/?gist=a52b7c8291573e677d89c01362d04f82

@jongacnik
Copy link
Member

jongacnik commented Nov 3, 2017

This is really cool (& nice dev experience). The delete instance on unload is a nice solve for cleanup!

Agree that registering components on application level is not ideal, and an approach which is decoupled from choo like this begins to feel more correct. Just figured I'd see what something like proposed api felt like since could be in place w/ component-box w/ very minimal code.

@bcomnes
Copy link
Collaborator

bcomnes commented Nov 3, 2017 via email

@yoshuawuyts
Copy link
Member Author

@bcomnes super keen to see what you come up with!


  • In my last project I have ~30 components, would I register all of them with the application? I imagine my application entry file looking like a total train wreck.
  • How would one load and register components async?

@tornqvist legit points!

I shoulda probably updated the original post a few days ago; last Saturday I met up with @nicknikolov and we talked about components. Also realized we're missing async loading, so updated it to support it. I thought I could make an example implementation sooner, but haven't had the time yet, so here's what I had in mind for an API:

index.js

var choo = require('choo')

var app = choo()
app.route('/', require('./views/main'))
app.mount('body')

components/button.js

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

module.exports = class Button extends component {
  createElement () {
    return html`<button onclick=${this._onclick}>
      Click me
    </button>`
  }

  update () {
    return false
  }

  _onclick (e) {
    this.emit('increment', 1)
  }
}

views/main.js

var Card = require('../elements/card')
var html = require('choo/html')

module.exports = function view (state, emit, c) {
  return html`<body>
    ${c('mapsCard', Card)}
    ${c('coolCard', Card)}
    ${c('listCard', Card)}
  </body>`
}

In this design, each component is still registered with a unique name (I'm not a fan of this), and instances can be addressed by that unique name. This also deals with async loading, which is the main difference with version I posted in the initial post.


Now this is from a few days ago; and if we can simplify the API further that'd be amazing (or like, come up with an alternate design that achieves the things better!)

Sorta just posting this in a flurry; think we're all on the same page we should definitely support async loading in our design! :D

@yoshuawuyts
Copy link
Member Author

@tornqvist ohh, I do like the way your proposal is going. The static method is rather elegant; quite like this approach!

@bcomnes
Copy link
Collaborator

bcomnes commented Nov 3, 2017

var Card = require('../elements/card') // These are just unmodified nanocomponents
var Item = require('../elements/item') // same
var html = require('choo/html')

module.exports = function view (state, emit, c) {
  return html`<body>
    ${c(Card).render(state, emit)}
    ${c(Card).render(state, emit)}
    ${c(Card).render(state, emit)}
    <ul>
       ${ data.map( d => html`
         <li>
           ${c(Item).key(d.id).render(d, emit)}
         </li>
       `)}
    </ul>
  </body>`
}

Since the top level passes down whatever c is, it can reference calls to c(Card) in order, so it can create and reference unkey'ed components in call order. For mapping data into components, you call c(Card).key('id') to get a key'ed reference to a component. The question becomes, how do we GC? Add unload hooks? Wait till idle in the browser and look?

@tornqvist
Copy link
Member

tornqvist commented Nov 5, 2017

The question becomes, how do we GC? Add unload hooks? Wait till idle in the browser and look?

I've been thinking about that as well. In my proposal I used the unload hook but don't know if that has any implications on performance? Also, it'd be nice if the user could override the GC for components that are very expensive to initialize and would want to stick around after being unmounted.


I couldn't help myself and made an implementation of the whole component registration thing, again using static methods. The static method register with the same signature as application stores (arguments being state and emitter) on the nanocomponent constructor would add on the emit handler to its prototype when called, much like how @jongacnik suggested. Also, for asynchronous registration, we'd add the register event that calls the register method of the given constructor.

Putting it all together, async components and all: https://choo-component-register.glitch.me
Source: https://glitch.com/edit/#!/choo-component-register?path=index.js:1:0

Pros

  • The public API of choo and nanocomponent remain unchanged (or rather, no breaking changes).
  • Asynchronous components would be registered using the event emitter, giving the whole app a chance to act on, and access the new component.
  • Registering 3rd party components with the application would use app.use, no need for a new interface.
app.use(require('some-3rd-party-component').register)
  • One can define a custom static register method and do all kinds of cool stuff now that the state and emitter are exposed.
class State extends Component {
  static register (state, emitter) {
    super.register(state, emitter)
    State.prototype.state = state
  }
  createElement () {
    return html`<pre>${JSON.stringify(this.state, null, 2)}</pre>`
  }
}

Cons

  • nanocomponent would be baked into choo. This is really just a design decision of mine so as users wouldn't have to register it manually. I don't know if it'd bring us over 4kb but if that's a pain point we could just encourage users to register choo/component themselves.
app.use(require('choo/component').register)

@jongacnik
Copy link
Member

jongacnik commented Nov 10, 2017

rad stuff @tornqvist. The register method here is a cool solve but somehow it's feeling a little verbose. Would be neat if could keep api as:

app.use(require('some-3rd-party-component'))

There could perhaps be some sort of check internally in the use method of choo which looks to see if argument has a register method, and if so, call it. Like:

// within choo itself, stripped all the asserts/timing for example sake
Choo.prototype.use = function (cb) {
  if (cb.register) {
    cb.register(this.state, this.emitter, this)
  } else {
    cb(this.state, this.emitter, this)
  }
}

This could perhaps keep the whole registration process more transparent and limits cognitive overhead? That said, it also makes some assumptions which I'm not sure is great?

For me there's still the larger question of if we need state/emit directly bound to components at all (vs just passing through props). If we don't, your initial proposal which simply takes care of identity is a direction I think still v cool!

@yoshuawuyts
Copy link
Member Author

@jongacnik the .use() API wouldn't work well with code splitting unfortunately :(

.use() is usually used inside index.js, so when loading the app for the first time, it would import all components. Instead each individual view should import the components they need, so we can split them and like everything would work out nicely (:

@tornqvist
Copy link
Member

@yoshuawuyts I take it @jongacnik was referencing my proposal of how to register components that you know you want included in the main bundle from the get-go. For async registration I proposed a new standard event register that'd work pretty much the same way; you'd just pass in the class and have its register method be called for you. So having the two methods of registering components (use/event) follow the same pattern of just passing in a class makes sense.

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 5, 2018

Class cache returns the instance, whatever it may be, which lets you call render or anything on it. This would work for yosh’s solution as well I think.

@jongacnik
Copy link
Member

jongacnik commented Jan 5, 2018

@bcomnes nice work on class-cache, excited to see these things moving along! the garbage collection stuff is 👌. fwiw component-box was/is also a generic cache class, nothing specific about components there just went with the name for the component™ 🤘

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 5, 2018

I might be wrong but the impression I got was it is a module level cache vs an instantiated cache. Not sure which is better or pros and cons at this point. But I think class cache is similar to component box with some subtle differences

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 5, 2018

We are talking about a call Sunday morning PST.

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 5, 2018

Sorry for the brevity, mobile at the moment

@jongacnik
Copy link
Member

yep, component-box is module level which was just a personal design decision to reduce boiler plate code based on how I was using it. Just var c = require('component-box') in a file and off to the races. That said, I think instantiated is better as a generic/universal solution (more robust, multiple caches, etc).

Noted on the call, may not be around but if so would be happy to jump on.

@yoshuawuyts
Copy link
Member Author

We had a call about this today: todo list from the call https://gist.github.com/yoshuawuyts/076a1e74b00f6729eaaba28f7ae1d249.

@rafaelrinaldi
Copy link

I really like the direction that this is taking. It’s probably not that relevant but maybe some ideas from data-components couls perhaps be useful from a component API design standpoint? I built it with very similar goals in mind.

@jongacnik
Copy link
Member

Rad to see you jump in here @rafaelrinaldi — been using data-components for years (thanks!). Have actually more recently found using it to wrap nanocomponents is nice for getting them into a page (esp when working w good old fashioned server rendered html). Some sort of higher level module for managing/auto-mounting nanocomponents to dom with learnings from data-components could be interesting as things move along.

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 25, 2018

I'll check out data-components looks cool

@kahlil
Copy link

kahlil commented Jan 29, 2018

👋 hi! I am sorry for barging in like this. I just saw that y'allz are working on Choo components in the latest blog post and was interested if you are considering using Custom Elements as a base model for them?

I went through the thread and only found that Custom Elements were quickly dismissed in a comment here. I believe that Custom Elements v1 are a fantastic browser-native component model which is supported by Chrome, Firefox, Safari, Safari for iOS, Chrome for Android and Samsung Internet. Others can be supported by conditional polyfills.

What's really neat about this:

  • Choo would have to ship less code (in most cases, depending on your audience) (:point_up: as you might have noticed above modern mobile browsers are fully supported so code is shaved in the right places)
  • Choo would be able to shave bytes off the code base over time for more browsers as the standard gets even wider support
  • The ChooComponentor whatever it would be called can just be a subclassed Custom Element and from what I have seen in this thread the APIs you are considering could easily be implemented with this standard.
  • Choo would use the platform even more which would improve it further because browser vendors are incentivized to improve support for Custom Elements
  • Choo could take advantage of Shadow Dom tech to also support isolated standalone components that are native to the platform

I think Custom Elements v1 are being overlooked and underestimated because it took so long for them to become something usable. But they are here now and I would really love to see more frameworks adopt them. Some excellent work using this standard has been done by @WebReflection with HyperHTMLElement of hyperHTML fame for example as well as in StencilJS.

Thoughts?

@bcomnes
Copy link
Collaborator

bcomnes commented Jan 29, 2018

Yeah custom elements are something I would like to explore sometime soon myself. Hyperhtml and the work by webreflections is very interesting.
I have some concerns about the global nature of registration and also the cost of json serialization in tight render loops but it does look promising.

@WebReflection
Copy link

WebReflection commented Jan 29, 2018

I have some concerns about the global nature of registration

there are discussions about having sandboxed CustomElementRegistry per shadow DOM but that's another story. Today both google amp and a-frame, together with others, used prefixes successfully. <choo-button /> doesn't look bad

the cost of json serialization in tight render loops

not sure what is this referring to but there's no automatic JSON serialisation in any of the hyper* libraries. apologies if it was referred to something else.

@kahlil
Copy link

kahlil commented Jan 29, 2018 via email

@WebReflection
Copy link

rich data is passed to Custom Elements via properties

watch out that's not always the case. CustomElements observable attributes passes through setAttribute and getAttribute involving stringification.

It's true that hyperHTML let you pass data=${anything} right away as special attribute as it's true that hyperHTML let you self close CE like JSX via <c-e /> but these are library features, unfortunately not how standards work.

@kahlil
Copy link

kahlil commented Jan 29, 2018 via email

@timwis
Copy link
Member

timwis commented Jan 29, 2018

Yeah it's one thing if you only use them in the context of choo because bel knows to set properties rather than just attributes (see my demo at the top of this thread) but if you try to use them as vanilla custom elements you'll have yo worry about serializing and deserializing if you want to pass objects.

@kahlil
Copy link

kahlil commented Jan 29, 2018 via email

@rafaelrinaldi
Copy link

@jongacnik @bcomnes Thank you for the input, glad it's helpful in any way. nanocomponent does look similar indeed.

@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Feb 3, 2018

@kahlil @WebReflection I kinda feel like the current version of Custom Elements is tricky to work with, for all reasons outlined earlier. But the thing that I'm personally hyper excited about is a formalization of bel/hyperhtml/lit-html through Apple's proposal!

I had some trouble reading the spec itself, but from the looks of it that would be the perfect to build on top of!

edit: @kahlil that said, it'd be really neat if at some point there was a way to construct Custom Elements as classes which could be instantiated as functions. Like: almost every other approach will have trouble rendering in Node; which means it's gonna be hard to use.

Also just properly tried out lit-html, and oh boy I'm really excited for this!

@brechtcs
Copy link

brechtcs commented Feb 9, 2018

I stumbled on this article by @pfrazee yesterday, and I think it might help us answer some of the issues you raised in your last comment, @yoshuawuyts. Specifically, I'm talking about the HTMLYoYo class he defines as a template for components. If you just export the inherited class as an npm module, you would basically give the user two choices:

  • Use them directly in a framework like Choo, which could call the render method to support things like server-side rendering.
  • Pass them into window.customElements.define, which gives you all the advantages of Web Components, while avoiding global scope problems.

@bcomnes
Copy link
Collaborator

bcomnes commented Feb 9, 2018

@brechtpm I've been wanting to experiment with that very idea but haven't had time.

@kahlil
Copy link

kahlil commented Feb 9, 2018 via email

@bcomnes
Copy link
Collaborator

bcomnes commented Feb 12, 2018

Finished class/nanocomponent-cache 1.x this weekend

I wanted do some comparisons between the 3 or 4 different implementations, but I'm not really actively solving this problem in my own work right now, so my motivation level is low on that.

That being said, I've been thinking about the identity method, and I'm still -1 on adding it by default, or to choo itself really. I think its the wrong level to be deciding how to 'key' up your components, and its the kind of default that will lead to weird bugs and confusion. The way react does its keys (the mapper decides how to key the components vs the component has a default key function) is already a point of confusion for newcomers, and covering up this important concept with a default value will only hide a key idea. Also, one of the things that appealed to me about choo was it was composed of independently assembled pieces. If we do add such a feature, I think it should be a store or feature you can activate rather than something that ships turned on, because I'm not sure I would use it myself. Also, we should avoid writing into a framework ecosystem when possible.

Some ideas that I do think would help improve the ergonomics of stateful components in choo:

  • Support components as choo view functions (either by supporting the .render function, or accept a component class, and instantiate on mount of the new view).
  • A better update function. Writing update functions is by far the hardest part of nanocomponent. Are there ways components can subscribe to data they want and only update when that data updates? Built in Vector clocks for choo stores?
  • An improved this.element system. The magic of nanocomponent is a query selector getter. I believe this fall out of the way isSameNode works. If we flesh out the concept of proxy nodes better and switch them over to using real DOM node references instead of an equality test, we could fix the morph leaks, and probably speed up access to nanocomponents by avoiding the queryselector and possibly enable keyless caching by render order.

Additionally, experimenting with what lit and web components are doing independently of nanocomponent, to come up with an improved alternative component model might be fruitful.

Also, if we manage to find consensus on this problem space, I have https://www.npmjs.com/package/component-cache which is a slightly more ergonomic name than nanocomponent-cache.

@yoshuawuyts
Copy link
Member Author

Updated choo-component-preview to v2.0.0. It now makes use of an LRU cache to evict entries, and has a more explicit API (no more static id method required), addressing some of the points @bcomnes raised.

If hope this strikes a good middleground between ease of use and having an explicit API.

var Article = require('./components/article')
var Header = require('./components/header')
var Footer = require('./components/footer')

module.exports = function (state, emit) {
  return html`
    <body>
      ${state.cache(Header, 'header').render()}
      ${state.articles.map(article => {
        return state.cache(Article, article.id).render(article)
      })}
      ${state.cache(Footer, 'footer').render()}
    </body>
  `
}

People on IRC seemed to be fairly into this; if people are good with it here too I might take a stab at updating #606 so we can think of landing this! 🎉

I'm also thinking of ways to address changes of this magnitude in the future. Discussion in this thread ended up with a long tail; perhaps having an explicit RFC proposal might help us focus on defining problems & solutions a bit better. Input on how to improve communication would be very welcome!

@bcomnes
Copy link
Collaborator

bcomnes commented Mar 7, 2018

:D

@YerkoPalma
Copy link
Member

Components are now availaible in choo, I guess is safe to close this, reopen otherwise.

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