MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
13.99k stars 926 forks source link

Object.create messes with component prototype hierarchy #1863

Closed sides closed 7 years ago

sides commented 7 years ago

Note: I use ES6 class syntactic sugar to manage component code.

When Object.create is called here it creates a new object using the component's instance as the prototype. This means that if a component has child components in its view method, but the child components change some of their properties in their view (e.g. via a bidi text box), they will be changing the property of this new topmost object, so when the parent component tries to read the property of its child it won't read the topmost property, but instead the unchanged original property of the prototype.

Example:

class PageComponent {
  constructor() {
    this.inputComponent = new InputComponent(); // stores the component before it gets morphed on init
  }

  view(vnode) {
    return <div>current value: {this.inputComponent.value} <br /> {m(this.inputComponent)}</div>;
  }
}

class InputComponent {
  constructor() {
    this.value = '';
  }

  view(vnode) {
    return <input type="text" oninput={m.withAttr('value', value => { this.value = value; })} value={this.value} />
  }
}

The "current value:" will not display whatever you type in the box. The inputComponent object will look something like this:

{
  value: 'whatever I typed in', // this gets created in the m.withAttr method
  __proto__: {
    value: '', // unchanged original value, what gets read in the PageComponent view
    __proto__: {
      constructor: InputComponent,
      ...
    }
  }
}

I tried using the new syntax, but it had the same problem.

spacejack commented 7 years ago

When using a class as a component, you need to write m(InputComponent... i.e., pass the class itself not an instance.

Trying to access another component's state directly is not a good idea. It would be better to pass a stream into the child component via attrs.

var stream = require ('mithril/stream')

class PageComponent {
  constructor() {
    this.value = stream()
  }

  view(vnode) {
    return <div>current value: {this.value()} <br /> {m(InputComponent, {value: this.value})}</div>
  }
}

class InputComponent {
  view(vnode) {
    return <input type="text" oninput={m.withAttr('value', vnode.attrs.value)} value={vnode.attrs.value()} />
  }
}
sides commented 7 years ago

I still prefer creating the instances myself and just using classes as a way to organize the code.

Might be my archaic thinking, but as for not accessing another component's state, out of curiosity, why not? I was going to make a form component that takes a dynamic amount of input components and the onsubmit would build values from the input components (via a getValue, not direct inputs). I can give them events instead whenever the value is changed that the form would configure, instead of doing it on onsubmit (like you're suggesting), but I'm failing to see the problem with the former.

Also curious as to why vnode.state is a copy of the component. It comes across as odd to me that a component inside its view method is a different object than the component itself.

sides commented 7 years ago

Also I'll close this to not clutter the list since my problem is solved. Would be interested to hear more though.

spacejack commented 7 years ago

It's so you can have multiple instances of the same component, eg:

view: () => [m('p', 'a:', m(Input, {a})), m('p', 'b:', m(Input, {b}))]

In my example, the value stream will update on key inputs (because withAttr is attached to the oninput handler.) So its value is always current and you can change how the form renders dynamically based on its value.

Alternately you could provide an onChange callback via attrs to the child and your parent component would be notified of the change. However a stream already does that with less boilerplate.

pygy commented 7 years ago

Mithril is made to work with ES5, and POJO components work as kind of classes with a constructor (oninit) and a destructor (onremove), but without builtin inheritance. The state object is equivalent to an instance.

The class components (really, constructible components) were added after the fact for people who want React-like class components. The class itself is the component, and the state literally is the instance.

If you use a class instance as component, as you noticed, Mithril treats it as a POJO component and uses it as the prototype of the state object.

dead-claudia commented 7 years ago

I'd have to say this is working as intended. In addition, if you changed your code to use class components directly rather than class instances as components, you might see it simplify some due to less boilerplate. (It's also more idiomatic.)

Edit: @sides

sides commented 7 years ago

Yeah, I'll have to review my current setup, which was just something I hacked together to work with the POJO style components (this was before support was added for ES6 classes). Thanks for the helpful responses.