-
Notifications
You must be signed in to change notification settings - Fork 6
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
Scaffold components #2
Comments
@choojs/trainspotters what would be a good boilerplate for components here?? I want to implement this. Is something like this ok? var Component = require('choo/component')
var html = require('choo/html')
class component extends Component {
constructor () {
super()
}
createElement () {
return html`
<div>
</div>
`
}
update () {
return false
}
}
module.exports = component |
That's what I would go for personally. |
Same, I like that! |
I feel like update returning |
There's no special reason to set it to false, so I could change it to true by default. |
LGTM! I think defaulting to |
Agree. So this might be a better default? var Component = require('choo/component')
var html = require('choo/html')
class component extends Component {
constructor () {
super()
}
createElement (state, emit) {
return html`
<div>
</div>
`
}
update (state, emit) {
return true
}
}
module.exports = component |
Assuming the component is to be rendered with the new component cache I think it'd make sense to reflect the signature imposed by the cache. It'd also help educate users on how to use the new component cache and encourage its use. The constructor will have the component cache constructor (id, state, emit) {
super(id)
this.local = state.components[id] = {}
} Idk if it'd make sense to assign |
yeah using the API that |
I'm ok with passing those arguments to the constructor, it will help a lot to folks like me :) I think is up to the developer how those are used, that's why I wouldn't assign them to |
So, for now component boilerplate would be like this $ choo-scaffold component custom-button var Component = require('choo/component')
var html = require('choo/html')
class CustomButton extends Component {
constructor (id, state, emit) {
super(id)
this.local = state.components[id] = {}
}
createElement (state, emit) {
return html`
<div>
</div>
`
}
update (state, emit) {
return true
}
}
module.exports = CustomButton if everyone is ok I will update the PR #6 |
PR updated 👍 |
This may be a little off-topic but I can't see why EDIT: sorry just saw the Still confused on intended usage, how much component has to know about cache, etc., but this is off-topic. |
@ungoldman The idea was to pass them to the constructor so that you wouldn't have to manually pass constructor (id, state, emit) {
super(id)
this.emit = emit
this.cache = state.cache
this.local = state.components[id] = {}
} |
@tornqvist you keep a cache reference in component to render other components? like component container pattern described in nanocomponent docs? |
That's right @YerkoPalma. Since the introduction of component cache in Choo core I only manually handle component instances when absolutely necessary and use |
Merged! but I can't publish 😶 |
@YerkoPalma yosh added it to the org just now, you should be able to publish :) |
@YerkoPalma you should be able to publish to npm, looks like you're on the access list: https://www.npmjs.com/package/choo-scaffold/access |
haha woops looks like @goto-bus-stop and yosh already took care of it at the exact time I looked 👀 |
haha nice! just got published as thank you all :) |
Once we settle on choojs/choo#593,
choo-scaffold
should be able to create components too. This is probably the main reason why this module exists in the first place, as creating components requires a fair bit of boilerplate.The text was updated successfully, but these errors were encountered: