Components #606
Components #606
Conversation
|
Oh, also note that this does not contain the PRs proposed in https://github.com/yoshuawuyts/choo-component-preview/pulls yet — we should merge those in before then. |
| // state | ||
| var _state = { | ||
| events: this._events, | ||
| components: {} |
There was a problem hiding this comment.
I noticed in the examples that this is being used for a kind of shared local state. Seeing as it doesn't tie in to the framework in any manner, it's easy enough to set up yourself and that choo in general is very unopinionated as to how one chooses structure state. Is there really a need for a default component property?
There was a problem hiding this comment.
You're right that there's not really a need for it, but it makes it just a little bit nicer if you choose to use it. Using a single state has lots of benefits: tracing, debugging, logging — it all becomes a lot easier.
Also, by naming it a particular thing, all applications now become a little more portable between each other. If you're writing multiple apps and sharing local state as global, it'll always be on the .components property of state.
I'm feeling this is kind of similar to window.initialState for server rendering — not essential, but having it in there clears up a line of decision making, which creates a more cohesive experience (:
There was a problem hiding this comment.
I'm totally on board with the one state-pattern, and now that you put it that way I guess it does make sense to have a default component property on there. I guess it's just a matter of properly documenting it as a recommended way of doing things.
index.js
Outdated
| var res = handler(self.state, function (eventName, data) { | ||
| self.emitter.emit(eventName, data) | ||
| }) | ||
| var res = handler(state, emit, cache) |
There was a problem hiding this comment.
I had this crazy idea while working with the preview. By adding just one line here, we can support top level components, as in component as view. If the talk of having async components (like the static getInitialProps in Next.js) were to be realized, and also with the trouble some newcomers have with figuring out async views, I figured it might be worth considering?
But really, this is more of a "why not?" than a proper suggestion. Just wanted to get it out there.
if (typeof handler.identity === 'function') res = cache(handler, state, emit, cache)There was a problem hiding this comment.
I'm not entirely sure what this code does. I'll have to take a look at the other PR I guess haha. I trust you it works — and fully subscribe to the benefits you're laying out here.
Also been thinking about Next.js style SSR rendering, and think once this patch lands we can work out the details to get some combination of events / class methods in there to make it streamlined. But this is probably first C:
edit: this is to say: feel free to push this change! — It'd be swell to have this working at the top level!
There was a problem hiding this comment.
My bad, I could have been more clear and have supplied a proper example. I added it in there now but didn't wanna update the todo example seeing as the main view wouldn't benefit from it in any way. But for the sake of documentation, this is what it would look like:
module.exports = class MainView extends Component {
static identity () {
return 'main-view'
}
update () {
return true
}
createElement (state, emit, render) {
return html`
<body>
<section class="todoapp">
${render(Header)}
${render(Todos)}
${render(Footer)}
</section>
${render(Info)}
</body>
`
}
}
|
About the GC, I implemented what we had in the preview over in my fork (tornqvist@783c5b1), should I push that here? |
|
@tornqvist yeah, please do push! :D |
|
Just a heads up, @yoshuawuyts. If this is to be the path forwards with components, we've got an issue with SSR. I was just implementing a version of this PR and realized that since edit: I solved it using a Proxy (only in Node). Idk if it's a feasible solution for choo but it got the job done… |
|
Replaced by #639 |
Adds https://github.com/yoshuawuyts/choo-component-preview/ to Choo. Semver minor. Updated the example too; seems to be a bit faster / easier to reason about (yay!)