liquid icon indicating copy to clipboard operation
liquid copied to clipboard

There should be a simpler syntax to define components.

Open Emasoft opened this issue 10 years ago • 54 comments

I tried it and I liked it (even if it is, unfortunately, non isomorphic), but I think that there should be a simpler syntax to define components. 59 lines to define a checkbox component? Really? This isn't gonna work. Simplification is the only way to make this working.

Emasoft avatar Nov 09 '14 01:11 Emasoft

I don't know any good way to make it "isomorphic" in Dart right now, but I've removed comment that it won't be isomorphic in the future. If Dart introduces something that it will be possible to make it work as easy as in Javascript, I'll add this feature.

Checkbox/TextInput components should be removed, there are easier way to implement them now. They were implemented because I've tried to implement all important features that React has, and in this case I just implemented them in "React-way" as Components.

Yes, there is a boilerplate for "virtual" constructor and "property" propagation, and I think that it should stay the way as it is for this low-level API and just create some other API on top of that, or maybe code generation with pub transformer. It would be also quite nice to have pub transformer for virtual dom, so it is possible to create virtual dom with something like v.tpl('<div id="x"></div>')

localvoid avatar Nov 09 '14 06:11 localvoid

Transformers are bad, because they destroy the rapidity and efficiency of a script language that doesn't need any compiling phase, like javascript. Dart should use transformers only for debug or for testing. If you want to make Liquid good, you need to avoid the use of any transformers. The boilerplate could be easily avoided automating things. There is nothing in the verbose syntax you are currently using for defining components that cannot be automatically inferred by some good heuristic code in the liquid lib. As my university professor always said: "The secret for good code is to never ask an human to decide anything that can be decided by the computer with a good algorithm, but to allow him to ovverride that decision at anytime".

Emasoft avatar Nov 27 '14 17:11 Emasoft

What do you think of the syntax/API that clean's react wrapper exposes: (using functions for components and 'native' tags)

see: https://github.com/cleandart/react-dart

div({"className": "somehing"}, [
  h1({"style": {"height": "20px"}}, "Headline"),
  a({"href":"something.com"}, "Something"),
  "Some text"
])

(obviously can also easily be namespaced if needed to sthg like r.div etc)

I like the idea of something like jsx for readability but all in all I also prefer to don't have transformers that aren't necessarily needed. For debugging something like the react chrome devtools extension would be nice:

https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en

Not sure how hard that would be though.

tosh avatar Nov 28 '14 00:11 tosh

I agree that transformers is a terrible idea.

I like clean's API, but how much performance and features we can sacrifice to make all this magic behind such API. For example, I don't want to treat classes like it is just a string, I want to add classes to the root element from the outside of the Component and from the inside (like it is done in Polymer). Applying styles is also a special case. So for this simple case we will need to make two get operations in the LinkedHashMap for each node, and as we know, string hashmaps in the dart2js world is slower then {} in js.

localvoid avatar Nov 28 '14 07:11 localvoid

I've looked at different ways on how to remove boilerplate with virtual dom nodes for components, and I can't find any reasonable way to implement it right now. There is even no support for varargs, and smoke doesn't support creating new instances by Type.

localvoid avatar Nov 29 '14 11:11 localvoid

I will add optional library `liquid/dynamic.dart' that will use mirrors, and API will look something like that:

class MyComponent extends Component { ... }

final vMyComponent = vComponentFactory(MyComponent);

class App extends Component {
  build() => vdom.Root([vMyComponent(#key, [arg1, arg2]]);
}

but it will work 6x times slower (vdom benchmark in Chrome).

localvoid avatar Nov 29 '14 12:11 localvoid

Mirrors are even worse! Please never use those if not for the debug version. You must use the classic object oriented way to create components, extending a base class. This base class can be bloated as you like, including all that the developers would ever need. In this way when they will subclass the base class for making a new component, they would only change some parameters and adding a very small number of methods. Include an autoconfiguration method in the base class, to set every member of the component automatically according to common usage patterns. Drop maps, use typed object properties. Use enums to give semantics to all kind of predefined usage patterns. Create a central, shared event manager, and make the base component already wired to it, so all the subclasses would have already an event system in place. They would only need to override an existing event handler if they need to change something. Think in a OO way, and you will make things simple and the code non redundant.

Emasoft avatar Nov 29 '14 17:11 Emasoft

Some thoughts/observations on API from playing with clean's react wrapper: (sorry if they don't make sense, I'm a bit jet-lagged atm) :)

  1. Maps vs JS Objects: It felt a bit weird (a bit of a regression to React.js) to use Dart Maps for props. I can't put my finger on it why, might just be that syntactically it is easier to parse when you have an object literal (clear differentiation between the key and the value (often a string) in comparison to having a Map with strings as keys.
div({style: {width: '100%', margin: '0 auto'}}, children);

vs

div({'style': {'width': '100%', 'margin': '0 auto'}}, children);
  1. Event Streams: I got used to dealing with events like change, input, mouseEnter etc as streams (I don't do heavy functional reactive stuff but occasional throttling, mapping, filtering). I guess the JavaScript API uses callbacks because there is no agreed upon streaming library that everyone uses (?) or callbacks might be 'good enough' for React components (?) you probably have more experience here.
div({onClick: this.handleClick}, children); // handleClick callback
div({onClick: this.clickStream}, children); // (?) not sure if this feels right, ideas?
  1. Props, clarity & validation: One thing I was missing a bit was that by looking at a component it isn't really visible what properties a component expects/requires.

It seems like there is some best practice (if you want to make props explicit) around listing all of the props you use at the beginning of the render method (?).

I guess part of this is because JS doesn't have support for named parameters and therefore everyone is (ab)using configuration objects.

The cool thing about this is that you can get started easily and it feels lightweight. If you have a data structure that is handed to your component that already has all the things you need you can just define a simple render method and you are finished. Nice for prototyping and you can make the component and its props more explicit later. Not sure what a nice and idiomatic Dart solution would look like here.

It is also neat that React has a concept of contracts/validation where you can optionally define what you expect of certain properties which goes beyond type checking.

Additional thoughts:

Obviously defining a nice and elegant API + considering performance in a dart2js scenario is quite a challenge here. Yet I have the feeling that potentially we should be able to take the React concepts and not only translate them into Dart but actually leverage idiomatic Dart to show how elegant React could have felt like if it was created in Dart.

(Sorry for the rambling, I hope some of the above thoughts make sense to you)

tosh avatar Nov 29 '14 17:11 tosh

I just saw how you expose/use streams in liquid via the component's element in the constructor:

http://localvoid.github.io/liquid/tutorial.html

tosh avatar Nov 29 '14 17:11 tosh

What about API for components like this:

class TodoItem extends Component {
  @property
  Item item;

  TodoItem(Context context) : super(context);

  build() => new VRoot([vdom.t(item.text)]);
}

final vTodoItem = vComponentFactory(TodoItem);

vTodoItem(#key, {#item: new Item('aaa')}); // will create virtual node for TodoItem component

It works right now using mirrors for development mode, and I can create a transformer that will generate all boilerplate in the most efficient way for production mode.

It will be possible to create different kinds of annotations for properties, like immutableProperty that will check for identity, and if it is identical it won't mark component as dirty. I think that this way will be much better then what it is implemented in React right now. Or annotations for property validation. And instead of implementing shouldComponentUpdate, user can implement custom setter that will check if change to this property invalidates the view.

localvoid avatar Nov 29 '14 18:11 localvoid

And to those who likes OOP inheritance I am almost implemented Component Decorators :)

class Button extends Paper {
  Ripple _ripple;
  html.DivElement content;

  @property
  bool disabled;

  html.DivElement get container => content;

  Button(Context context) : super(context);

  void create() {
    super.create();
    _ripple = new Ripple(this);

    element.classes.add('mui-paper-button');

    content = new html.DivElement()
      ..classes.add('mui-paper-button-content');

    inner
      ..append(_ripple.element)
      ..append(content);

    element.onMouseDown.listen(handleMouseDown);
  }

  build() {
    super.build().decorate(vRootDecorator(null, classes: disabled ? ['mui-disabled'] : null));
  }

  void handleMouseDown(html.MouseEvent ev) {
    var x = ev.offset.x / element.clientWidth;
    var y = ev.offset.y / element.clientHeight;
    _ripple.animate(x, y);
  }
}

final vButton = vComponentContainerFactory(Button);

and now it is possible to create Button like this:

vButton(#key, [v.t('MyButton')]);

or subclass it

class MyButton extends Button {
  MyButton(Context context) : super(context);

  build() {
    super.build().decorate(vRoot([v.t('MyButton')]));
  }
}

localvoid avatar Nov 29 '14 18:11 localvoid

Please avoid mirrors and transformers at all costs. Use those only for debug or tests. The moment you use one of those two, people will be forced to stop using your library. I and I want to use it, because I like it very much. You should keep it slim and rapid. There are many ways to solve those problems without using mirrors and transformers.

Emasoft avatar Nov 29 '14 18:11 Emasoft

I don't see any problems with mirrors for development mode and replacing them with transformers for production. Polymer and AngularDart using this model.

localvoid avatar Nov 29 '14 19:11 localvoid

Even if you don't use mirrors in production mode, you should avoid using a different technique in production mode and in development mode. It is a recipe for unexpected bugs. I use mirrors only in tests. Transformers force us to recompile every time we change something, and this cancels the usefulness of using a script language. Changing something and being able to immediately see the effects is very important. I don't use Polymer and Angular for this reason. I need a development framework lean and quick. This is the thing that makes javascript much loved even on the desktop, where node.js is gaining traction. It seems a small thing at the beginning, but when the project grows you start to appreciate it.

Emasoft avatar Nov 29 '14 20:11 Emasoft

I don't see any other way to implement good API, and all other solutions that I can think of is way much worse.

Javascript world is moving toward ES6/7 compilers like traceur, TypeScript, CoffeeScript etc. Even with old javascript most of the projects are using different preprocessors + gulp/grunt etc. The same on the desktop, projects like Atom are using CoffeeScript, etc.

localvoid avatar Nov 29 '14 20:11 localvoid

Didn't tried it yet, maybe use call() method for children.

v.div(#key, styles: {'width': '100%', 'margin': '0 auto'})([
  v.div(1)('line one'),
  v.div(2)('line two')
]);

localvoid avatar Nov 29 '14 22:11 localvoid

If I learned something in my life as a programmer is that there is always a way. Give me a couple of days to study this. I'll be back to you with some ideas.

Emasoft avatar Nov 29 '14 23:11 Emasoft

Implemented new API for basic virtual dom elements, didn't notice any performance regressions in the VDom Benchmark.

v.div(#key, styles: {'width': '100%', 'margin': '0 auto'})([
  v.div(1)('line one'),
  v.div(2)('line two')
]);

localvoid avatar Nov 30 '14 05:11 localvoid

Looks nice, what if I have an empty element? Would it then be v.div(1) or v.div(1)()?

Is there a reason for having key as positional (& mandatory?) parameter? Can it be an optional named parameter as well?

tosh avatar Nov 30 '14 06:11 tosh

Just v.div(1) for empty element.

I thought about making it implicit, and it is a tough choice. If I make it implicit, there will be different kinds of problems for people who doesn't understand how vdom works, and there will be tons of questions why it doesn't work as they expect.

In the React they've chose to make it implicit and in the dev mode they just write in the console that keys should be added when there is something terribly wrong, yet there are still many confusion about it for newcomers.

localvoid avatar Nov 30 '14 07:11 localvoid

Very elegant using call.

It is a tough decision. I don't know enough about the diffing & vdom yet to fully understand all the trade-offs.

I expect I personally will often want to use immutable data structures if possible so having an API where the key is mandatory even if I just fill it out with a dummy key is a bit weird. Should I still provide keys in this case? Would I need to make sure to use the same keys for nodes that have the same props in them?

Is there a way to automatically generate unique keys for every element or would that mean that nodes which might actually be practically the same to no longer be regarded as the same and therefore adding more diffing work and memory consumption?

tosh avatar Nov 30 '14 07:11 tosh

To create complex web apps, we will need to create stateful components like this button, and it doesn't matter which way we choose to do it, with WebComponents that described in the futurice articles, or with lightweight liquid components.

So, everytime we rerender list of this buttons, we need to preserve state for ripples and make sure that this ripples doesn't migrate to other button when we add one more button at the beginning. It is the most important case, and the second one is that this way you can guarantee "minimal" number of dom mutations, because even simple changes with implicit keys will result in destroying all performance.

Implicit keys are good for static trees, when nothing is changed, but I prefer to use real dom to create static trees, and I am using virtual dom only for mutations (Button example) and keys is just an easy way to control this mutations.

Immutable data structures doesn't change anything in this case, everytime you have list of entities, you need to create unique key for each entity in this list. It is like primary key in databases, it doesn't matter what data structures beneath it(mutable or immutable), entity will always have some unique key that identifies it. And VirtualDOM node key is used to create a relationship between node and this entity.

I think that if you try to render simple immutable list in React, and then add one item to it, React will tell you that you should add unique keys to your entities in immutable list, but I am not sure when they start to warn users that they're doing it wrong.

localvoid avatar Nov 30 '14 08:11 localvoid

It is also possible to implement hashCode and operator== in your entity, and use it as a key for vdom node.

localvoid avatar Nov 30 '14 08:11 localvoid

Thanks for your explanation, here is the related React documentation just for reference:

http://facebook.github.io/react/docs/multiple-components.html

I think your decision to have it explicitly in the API is wise because users need to understand and know about it. On the other hand I personally still prefer an API that only on-demand reflects the complexity of the case I'm dealing with allowing me to not provide keys if they are not needed.

If we zoom out a bit conceptually what I don't like about key being in the API (and having to be documented and explained) is because it is an implementation detail of diffing stateful components that leaks into what the user of the lib has to manage. Therefore the developer can no longer ignore how the diffing works. The developer has to watch out for internal state making the solution more complex.

This might be a bit radical but I like quiescent's take on local state:

https://github.com/levand/quiescent (see the readme on differences to Om)

If state of a component is not located/hidden/stored in the component itself (meaning it has to be passed in again) makes the rendering way easier to understand and consistent to what usually happens (conceptually just 'dumb' re-rendering, obviously this is not what happens but what actually happens is a performance implementation detail I don't have to know about then).

The disadvantage is (like David Nolen of Om says) is that it causes state that would otherwise be part of the component itself to leak outside of it 'polluting' application data (that said it is possible to make things explicit and have a separation between business data and ui state data).

tosh avatar Nov 30 '14 10:11 tosh

In other words It seems like quiescent is trading locality/encapsulation for simplicity. Yet the approach is still quite modular as you can easily mount and use the components wherever you want and they control their own behaviour and business logic, you just have to provide them with all their state (application data & ui data).

On a scale this would move the hypothetical component I'm describing between React & vdom

(local: state & logic)    (external: state, local: logic)    (external: state & logic)
React <=============================> Liquid <=============================>  vdom

tosh avatar Nov 30 '14 10:11 tosh

quiescent still seems to require the key information. TodoMVC example: https://github.com/levand/todomvc/blob/gh-pages/architecture-examples/quiescent/src/todomvc_quiescent/render.cljs#L74)

I think I don't understand the diffing/reconciliation good enough yet. (I need to go through it). http://facebook.github.io/react/docs/reconciliation.html

That said I feel the ClojureScript projects that leverage React (Om, quiescent, reagent) are a better source for inspiration than React itself because of their simplicity.

I'm also looking at Mithril and a couple of others that seem to improve on React.

tosh avatar Nov 30 '14 11:11 tosh

The whole point of vdom is to easily manage stateful trees, otherwise it is just a terrible "performance optimization".

Just imagine simple user list, and now you want to render it in two views, each user list view is a list of buttons, each button has a ripple effect. In this stateless ui model the easiest way will be to add state for each button to the user entities, but it is wrong in so many ways. So now you need to create ViewModel for each view that will have state for ripple effects of all buttons, and now you need some way to synchronize data between Model and ViewModel and now you're implementing data-binding, or something else.

http://futurice.com/blog/reactive-mvc-and-the-virtual-dom/ "No reusable UI components. React has a strong emphasis on "reusable UI components", which is unmatched in this current MVI proposal."

The idea of pure stateless UI components is so old, and I don't understand why it is hyped right now, it just doesn't work for general-purpose UI libraries that is all about building reusable UI components to reduce complexity when building large applications.

localvoid avatar Nov 30 '14 11:11 localvoid

Without keys you just rerender the whole real DOM from scratch when something is changed. It is the easiest way to think about it, because algorithm for implicit keys is dumb and works only for static trees.

localvoid avatar Nov 30 '14 11:11 localvoid

I just read through the reconciliation documentation. Interesting heuristics.

I now also get why we can't just have the component generate a unique key upon creation because the key needs to be stable to make it possible to recognize a component when lists of children are compared (main use-case like you mentioned is new insertion at the top of the list).

I think usually when you have a component (let's say UserAvatar) the component itself should be able to pick a stable key for itself from its props (eg in this case the id of the User might work well).

Wouldn't it feel more natural to have the component be responsible for this instead of the owner?

One advantage of allowing the owner to be responsible is that if a component author didn't think of their component being used in a list of children the owner component can still set keys.

How about having key as optional named parameter (therefore allowing it to be set from the outside upon creation) as well as having a way to define it internally (by overwriting the attribute or defining a getter)?

That way the API is clean and simple except in the rare use-case when a component is used many times in a list and did not internally define a key.

tosh avatar Nov 30 '14 13:11 tosh

It is actually a good idea, it is possible only as a static method, or using some annotation that will mark one of the properties as a key, because we don't have access to the component instance at this moment.

But it will be a problem for components like buttons, because they don't have properties from which they can generate unique keys, and basic vdom elements wouldn't have the ability to guess keys, so there will be inconsistency between elements and different kinds of components.

localvoid avatar Nov 30 '14 14:11 localvoid