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

Fix docs bug, advise against reusing vnode.attrs itself #2250

Merged
merged 5 commits into from
Nov 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/change-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
- render/events: `Object.prototype` properties can no longer interfere with event listener calls.
- render/events: Event handlers, when set to literally `undefined` (or any non-function), are now correctly removed.
- render/hooks: fixed an ommission that caused `oninit` to be called unnecessarily in some cases [#1992](https://github.com/MithrilJS/mithril.js/issues/1992)
- docs: tweaks: ([#2104](https://github.com/MithrilJS/mithril.js/pull/2104) [@mikeyb](https://github.com/mikeyb), [#2205](https://github.com/MithrilJS/mithril.js/pull/2205), [@cavemansspa](https://github.com/cavemansspa), [#2265](https://github.com/MithrilJS/mithril.js/pull/2265), [@isiahmeadows](https://github.com/isiahmeadows))
- docs: tweaks: ([#2104](https://github.com/MithrilJS/mithril.js/pull/2104) [@mikeyb](https://github.com/mikeyb), [#2205](https://github.com/MithrilJS/mithril.js/pull/2205), [@cavemansspa](https://github.com/cavemansspa), [#2250](https://github.com/MithrilJS/mithril.js/pull/2250) [@isiahmeadows](https://github.com/isiahmeadows), [#2265](https://github.com/MithrilJS/mithril.js/pull/2265), [@isiahmeadows](https://github.com/isiahmeadows))
- render/core: avoid touching `Object.prototype.__proto__` setter with `key: "__proto__"` in certain situations ([#2251](https://github.com/MithrilJS/mithril.js/pull/2251))
- render/core: Vnodes stored in the dom node supplied to `m.render()` are now normalized [#2266](https://github.com/MithrilJS/mithril.js/pull/2266)
- render/core: CSS vars can now be specified in `{style}` attributes [#2192](https://github.com/MithrilJS/mithril.js/pull/2192)
Expand Down
66 changes: 65 additions & 1 deletion docs/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function ComponentWithState(initialVnode) {
// Component state variable, unique to each instance
var count = 0

// POJO component instance: any object with a
// POJO component instance: any object with a
// view function which returns a vnode
return {
oninit: function(vnode){
Expand Down Expand Up @@ -431,6 +431,70 @@ This way, the `Auth` module is now the source of truth for auth-related state, a

As a bonus, notice that we no longer need to use `.bind` to keep a reference to the state for the component's event handlers.

#### Don't forward `vnode.attrs` itself to other vnodes

Sometimes, you might want to keep an interface flexible and your implementation simpler by forwarding attributes to a particular child component or element, in this case [Bootstrap's modal](https://getbootstrap.com/docs/4.1/components/modal/). It might be tempting to forward a vnode's attributes like this:

```javascript
// AVOID
var Modal = {
// ...
view: function(vnode) {
return m(".modal[tabindex=-1][role=dialog]", vnode.attrs, [
// forwarding `vnode.attrs` here ^
// ...
])
}
}
```

If you do it like above, you could run into issues when using it:

```js
var MyModal = {
view: function() {
return m(Modal, {
// This toggles it twice, so it doesn't show
onupdate: function(vnode) {
if (toggle) $(vnode.dom).modal("toggle")
}
}, [
// ...
])
}
}
```

Instead, you should forward *single* attributes into vnodes:

```js
// PREFER
var Modal = {
// ...
view: function(vnode) {
return m(".modal[tabindex=-1][role=dialog]", vnode.attrs.attrs, [
// forwarding `attrs:` here ^
// ...
])
}
}

// Example
var MyModal = {
view: function() {
return m(Modal, {
attrs: {
// This toggles it once
onupdate: function(vnode) {
if (toggle) $(vnode.dom).modal("toggle")
}
},
// ...
})
}
}
```

#### Don't manipulate `children`

If a component is opinionated in how it applies attributes or children, you should switch to using custom attributes.
Expand Down