meteor / guide

:book: Articles about Meteor best practices
http://guide.meteor.com/
Other
840 stars 305 forks source link

Should we recommend Blaze Components? #8

Closed stubailo closed 3 years ago

stubailo commented 9 years ago

I think it's pretty clear when you take a look at the application development landscape these days that people are pretty excited about reusable components. Blaze is a great templating system, but it doesn't quite give you everything you need to be able to reason about small parts of your app independently.

Mainly the issue is that it can be hard to look at a Blaze template and know how it will behave when used in different parts of your app. This involves a few issues:

  1. The tendency to use global data via Session - partly because using Template.instance() all the time is a lot of boilerplate
  2. The tendency to use global jQuery selectors, or HTML IDs, like $('.error')
  3. The data context being passed from outside of the template means the functionality depends on the outside environment in a poorly-defined way
  4. The way event maps work means they also catch events from child templates, which is something you might or might not want

@mitar, can you help me understand to what extent your Blaze Components package solves these issues? Unfortunately, you're right - I haven't had enough time to look at it in depth, but I would like to learn more now.

Copied below, the outline of the Blaze guide as proposed:


Blaze guide: The Tracker-based reactive templating system

Write “HTML with holes” just like you're used to, and get a fast, fine-grained, reactively updating page with no sweat.

  1. Spacebars syntax, and how to use the built-in helpers
  2. Building reusable components with Blaze
  3. How to use reactivity in a principled way
  4. Writing maintainable helpers and event handlers that aren't tightly coupled to HTML
  5. Reusing logic and HTML snippets between templates
Slava commented 9 years ago

A reusable global helper {{instance}} described by david weldon (https://dweldon.silvrback.com/template-instance) is pretty useful too in combination with Template.instance(). Do you think it is a good pattern to recommend in this guide?

stubailo commented 9 years ago

Ooh, yeah - that sounds really awesome.

mitar commented 9 years ago

I think you should read these two posts:

The tendency to use global data via Session - partly because using Template.instance() all the time is a lot of boilerplate

Yes, this is definitely fixed, because you can just use this.

The tendency to use global jQuery selectors, or HTML IDs, like $('.error')

To do what? For event handlers? So I think it is OK to use jQuery selector to read data, but you should not be modifying DOM through jQuery. For the second point, it makes it easier to have local state which Blaze can then use to modify DOM, so you do not have to use jQuery to do that. But for reading DOM stuff (like form input values) you still use jQuery. If you want that one changed, then look at the ViewModel. Which could also be just an extended base class or Mixin for Blaze Components. But for me I wanted something really core. Things like ViewModel are for me something which should be opt-in built on top of Blaze Components.

So Blaze Components support nice OOP extending, so you can edit such goodies then onto the pretty basic core.

The data context being passed from outside of the template means the functionality depends on the outside environment in a poorly-defined way

Please read the forum posts about this. Because I do not see this as big problem. I see that data context is something which is the data passed from the tree hierarchy, which I think it is a good thing, like the context in which a component is. Why is this poorly-defined? I think it is great. The issue is only if this is the only way how templates can get some configuration, and then user start modifying data context locally to pass stuff to the template.

So in Blaze Components you have three ways to get data:

The difference between first two is that component is not recreated if data context changes, but it is if arguments change. So arguments is something you pass when including a Blaze Component:

{{> FooComponent args color='blue'}}

So the idea is, data context is something which is reactively changing, and component instance can keep the state while things get rerendered (like which entry is visually expanded).

But, I think the important to understand is that by the changes made for Blaze Components, is pretty easy to add some other ways to get data. So I think core should be kept simple, but then there will probably be some other ways community will build helpers on top (as base classes or as mixins). For example, you can see in the second form post a way to have communication between component instances by exposing some type of API. You can imagine that you can use that also to provide some other ways to distribute data.

So I am trying to keep Blaze Components without many opinionated decisions (I think there are still many there), because this is also something what practice will show. For now it just fixes some issues I had with Blaze and makes it so that things are reusable.

The way event maps work means they also catch events from child templates, which is something you might or might not want

This is not something we actively prevent, but it is easy to do so in the event handler:

onClick: function (event) {
  // If the location where the event originated is not in this component, skip it.
  if (this.component() !== this.currentComponent()) return;

  // The rest.
}

So you have access to currentComponent() which is the component where the event originated, and you can verify if this is the same as component() which is where you are defining the event handler.

BTW, you also have data() and currentData() which is the same story for data context.

A reusable global helper {{instance}}

That exists in Blaze Component for free. Because every method is available as a helper in the template, then simply calling {{component}} or {{currentComponent}} allows you to access the component. :-)

In fact, Blaze Components drastically minimizes global helpers because you can simply create a method on a base class which all your components are using and it will be available to all your child components. And you can even extend them if needed in some. I do not have to write global helpers anymore at all.

@stubailo, I really think you should try Blaze Components for a few days. :-)

stubailo commented 9 years ago

To do what? For event handlers? So I think it is OK to use jQuery selector to read data, but you should not be modifying DOM through jQuery.

The issue I'm referring to is that if you just do $('.error') in your code, you select every single element on the page with that class. I don't see people using this.$ at all, which is a big part of having components be reusable.

I see that data context is something which is the data passed from the tree hierarchy, which I think it is a good thing, like the context in which a component is.

I think it's a bit like this in JavaScript - yes, people who are very experienced know where it comes from, but I don't think anything is lost by getting rid of it entirely and passing arguments instead. I think React really really got it right here - being able to document, in the code, what data your component expects to get is immensely valuable. There's a reason people prefer to have real function signatures in JavaScript instead of mangling the arguments pseudo-array all the time.

Maybe what I really want is an explicit way of validating what context the component expects, so that you get an error if you accidentally pass the wrong context. I think this is the main issue - if you just see a {{> MyComponent}} somewhere, that doesn't mean it is taking no arguments! It means that it's inheriting the context of the enclosing element, which might or might not be what you expect.

@stubailo, I really think you should try Blaze Components for a few days. :-)

Hey, I have read a lot of your design stuff and I like it a lot. It addresses a lot of things I would have wanted to build and put in the Meteor core. Unfortunately I probably don't have time at the moment to sit down and use the library for an extended period of time so I'm drawing on some of my previous experience facing these problems.

I'd also love to get some people to chime in on other options like Flow Components, and ViewModel. I am personally not a fan of bringing in two-way data binding, especially since even Angular might be moving away from it, but I'd like to hear both sides of the issue. I think I can be much more efficient if I can moderate between different points of view rather than going our and directly researching everything myself. Plus, if we get it wrong, we can always fix the guide later.

mitar commented 9 years ago

The issue I'm referring to is that if you just do $('.error') in your code, you select every single element on the page with that class. I don't see people using this.$ at all, which is a big part of having components be reusable.

Oh, this is because currently you have to do Template.instance().$, I would guess.

In CoffeeScript is really simple, @$('.error'). :-) Only one character more. :-)

But I think once you have components and you know that you want to make them reusable, it is easy to start using this.$. It is there and it is for free. I think this is similar to how people start using this.subscribe and this.autorun. It is also something which we should be pointing out in various snippets around the web to fix them.

I think it's a bit like this in JavaScript - yes, people who are very experienced know where it comes from, but I don't think anything is lost by getting rid of it entirely and passing arguments instead.

OK, you have to understand, in Blaze Components it is something which is not the main thing around everything is centered. You can use them completely without data context. And if you do use data context, then you can access it by calling this.data() (and only then a reactive dependency is made). This is it. It does not magically change anything under your feet.

So I think the approach of Blaze Components is good. If you do not want to use data context, you do not have to. If you do want to, you can. It is just one way to pass data to the component.

There is one place where Blaze Components are ugly and this is how you are passing arguments to the component. Internally we still have to use data context for that (and a special helper args). So there would be some opportunity to improve.

I think React really really got it right here - being able to document, in the code, what data your component expects to get is immensely valuable.

I think it is even more useful (and more Meteor-like) to have components provide (reactive) APIs to each other and then you can use that. Your component needs some data from the parent? this.componentParent().highlightedId(), done. I am not sure if tree-like dependencies (parent providing to children) are enough in complex UI systems. Like we implemented windowing system on top of Blaze Components, no way you can do that by just providing arguments from top to bottom. Sometimes children has to talk to parent "hey, I am expanding".

And also, I do not think it is a good idea to try to make Blaze like React. If people prefer how React is doing things, then they can use React. And leave Blaze to those who prefer how Blaze is doing things. So I think some of these questions are matter of style and personal preferences. It feels a bit like discussion between strict and dynamic typing. You cannot have both. And you are saying React is more strict, let's move there. And I am saying, no, leave our dynamic language as it is, those who ant to use strict one can use React. But it is good to also have a dynamic one around for those who prefer this one.

So how I would like to see it:

And people can choose. I do not think there is one ultimate design here. What Blaze Components are trying to achieve though is keeping with Blaze design (data context, event handlers, reactivity as the main way to communicate state changes), but make things reusable and extendable so that community can start building on top of that new patterns, new insights, new ideas (like MVVP pattern).

Maybe what I really want is an explicit way of validating what context the component expects, so that you get an error if you accidentally pass the wrong context.

Oh, I should read further. Exactly. You want to move programming language to a more stricter one. This is really a bad thing to do once a language is released. :-) People are probably using it for this particular reason as well, so it is easier for them to use something where this is core design from the beginning, if they really want it.

I think that is maybe just a question of tooling. So for dynamic languages you then have tests, and inspectors and so on. Maybe this is what is missing. Let's leave Blaze dynamic, but you can then use things like Meteor toys to inspect what template is getting as a data context.

Or we could define tests for templates which would test them with different data contexts.

I think we should investigate that approach more.

if you just see a {{> MyComponent}} somewhere, that doesn't mean it is taking no arguments!

And then your IDE can in real-time show you what is being passed and if the MyComponent calls data() or not. ;-)

Flow Components

I am not sure how much they have been developed, I think @arunoda moved to React?

ViewModel

I think this is a great approach as a extension to Blaze design. Where Blaze Components just try to wrap Blaze into a reusable and extendable system, but keeping design as it is, ViewModel extends the design with MVVP. I am not sure if this is something which should be the only way to do it, but it is a great innovation and I think this is an example of an innovation I would like to see more on top of Blaze once we make it reusable and extendable. :-)

mitar commented 9 years ago

Unfortunately I probably don't have time at the moment to sit down and use the library for an extended period of time so I'm drawing on some of my previous experience facing these problems.

They are completely interoperable with existing templates. So you can just decide to make one view with it in your existing projects. (Like Galaxy. ;-) )

stubailo commented 9 years ago

@mitar is there a good canonical example app with Blaze Components I can look at?

mitar commented 9 years ago

I think Wekan is using them a lot, but I am not sure what patterns are they using. But from what I am seeing I think they are doing it right. Our own stuff using them is sadly not yet open source, nor public.

ghost commented 9 years ago

Maybe what I really want is an explicit way of validating what context the component expects, so that you get an error if you accidentally pass the wrong context.

I think the pattern that we can recommend for Blaze is:

template.onCreated(function () {
  const self = Template.instance();
  check(self.data, {
    attribute1: String,
    attribute2: Number
  });
});

I use const self = Template.instance(); because it is the consistent way to get the template instance in any part of the template.

ghost commented 9 years ago

Here is my current idea of reusable components just with Blaze. This is basically the boilerplate code that you would use to start a new component. It includes the following parts of the component:

const template = Template.myComponent;

template.onCreated(function validateArguments() {
  const self = Template.instance();

  check(self.data, {
    initialValue: String
  });
});

template.onCreated(function defineState() {
  const self = Template.instance();

  self.myValue = new ReactiveVar(self.data.initialValue);
});

template.onCreated(function defineApi() {
  const self = Template.instance();

  self.doSomething = function (newValue) {
    self.myValue.set(newValue);
  };
});

template.helpers({
  myValue() {
    const self = Template.instance();

    return self.myValue.get();
  }
});

template.events({
  'click .my-button': function () {
    const self = Template.instance();

    const newValue = self.$('.my-input').val();
    self.doSomething(newValue);
  }
});

Using this structure consistently makes is reasonable easy to work on such components that are defined with Blaze. Each aspect of the template has its own part.

What I like about this approach compared to larger component packages like Blaze Components or flow-components is that you only need core Meteor APIs (Blaze, check and ReactiveVar), so it is easier to understand I think, because you don't have to learn a new 3rd party package before you can understand the code. I think that is a good trade off for the higher verbosity that this code has compared to Blaze Components.

I use const self = Template.instance(); because it is the consistent way to get the template instance in any part of the template. So you don't have to think about how to access the instance in different parts of the template.

tmeasday commented 9 years ago

One idea that I've discussed a little with Sacha and I'd be surprised if no one else is doing is that you can actually get some useful React-style lifecycle methods + "props" checks via the use of the .onCreated() callback.

Like for instance:

Template.prototype.dataMatch = function(types) { // equivalent to propTypes
  // here this is the template, eg Template.foo
  this.onCreated(function() {
    // here this is the template instance
    check(this.data, types); // say, or log a react-style warning
  });
};

// usage
Template.myTemplate.dataTypes({
  post: Object,
  showAuthor: Boolean
});

Or another useful one, maybe

// equivalent to componentWillReceiveProps
Template.prototype.templateWillReceiveData = function(cb) { 
  this.onCreated(function() {
    let oldData = this.data;
    this.autorun(() => {
      const newData = Template.currentData();
      cb(oldData, newData);
      oldData = newData;
    });
  });
};

(Note this is just spitballing but I know the technique works).

I feel like the combination of: a) this b) things like {{instance}} + something that wraps Template.prototype.helpers() to make this the template instance. c) some really good patterns around attaching reactive dicts to instances and using them for state.

And a lot of the objections to Blaze would be reduced.

I know that something like Blaze Components does all this and more but I guess my biggest concern with a library like that is that there is so much going on there that it's a big surface area to be maintained. From experience with IR it's difficult to maintain a large and complex project when everyone in the community starts using it!

Anyway, as I said, just spitballing but I definitely think there's value in a "how to use Blaze better" guide.

tmeasday commented 9 years ago

(Basically I am in agreement with @Sanjo I think ;) )

stubailo commented 9 years ago

@tmeasday @Sanjo are there any existing minimal packages that reduce the amount of boilerplate necessary to get this stuff right?

my biggest concern with a library like that is that there is so much going on there that it's a big surface area to be maintained

I am inclined to agree with this - and if the original maintainer isn't interested anymore, it's hard for someone else to pick it up if it's a big chunk of code.

So should we document the Blaze boilerplate first, and then at some undefined later date write some code to reduce it? Maybe we can even have that be a callout in the guide like "hey look at this boilerplate! you could be the one to fix it! your package here" kinda deal.

ghost commented 9 years ago

@tmeasday @Sanjo are there any existing minimal packages that reduce the amount of boilerplate necessary to get this stuff right?

I guess Blaze Components does this. (Disclaimer: I still haven't worked extensively with this package but Mitar is in this topic anyway).

@mitar Could you provide the equivalent of the code in https://github.com/meteor/guide/issues/8#issuecomment-146110276 for when you write it with Blaze Components? Then we have a one to one comparison.

This is out of the scope of this guide but: I think someone at MDG should look at the question if Blaze Components already solves all the open issues with components and Blaze and if that's the case it should be made part of the core ASAP. Because right now Blaze is not on a par with React (and probably Angular 2) when it comes to components.

Making Blaze Components part of the core gives the strongest indicator from MDG that this is something, that is recommended and is supported by MDG. It becomes the standard way for components with Blaze.

mitar commented 9 years ago

I know that something like Blaze Components does all this and more but I guess my biggest concern with a library like that is that there is so much going on there that it's a big surface area to be maintained.

It is 700 lines of code. I do not think that is a big surface area to be maintained? It is really mostly just wrappers around Blaze. So all the complicated stuff is taken care by Blaze. Blaze Components just expose a different API on top of it. The only slightly messy part is passing arguments to components.

mitar commented 9 years ago

are there any existing minimal packages that reduce the amount of boilerplate necessary to get this stuff right?

Yes, Blaze Components. I know that it looks magically how much they can do, but it is really just putting things in the right way together and then things empower each other.

So should we document the Blaze boilerplate first, and then at some undefined later date write some code to reduce it? Maybe we can even have that be a callout in the guide like "hey look at this boilerplate! you could be the one to fix it! your package here" kinda deal.

I think this is exactly what Blaze Components are about.

So I would say that there are two goals with Blaze Components:

I think that prior comments are were mostly about the first point. But I am looking further, once you fix the first problem, then the second becomes important. You want to be able to be able to share components easily and be able to change things slightly, if necessary. You want to be able to compose things together. Because of current state of Blaze it is really hard to reuse templates. But you can do that with Blaze Components.

I would say that third of the codebase deals with this second point. If there is big interest, it is really easy to split it out. But I do think it is important to provide "official" way to compose components and extend them so that things can inter-operate.

mitar commented 9 years ago

and if the original maintainer isn't interested anymore, it's hard for someone else to pick it up if it's a big chunk of code.

OK, but this will be the issue with any package presented in the guide. Maybe MDG could think about various ways to encourage that this does not happen:

Maybe that could be an interesting idea. That we could establish a group of non-core developers who would be secondary maintainers of all packages promoted in the guide. Something similar they did for Trac: https://github.com/trac-hacks So maintainers can move their package into a common project space, so that in the case that maintainers disappear, project as a whole can then find a new maintainer and give permissions to maintain.

stubailo commented 9 years ago

@mitar what's the test coverage like for Blaze Components?

mitar commented 9 years ago

@mitar what's the test coverage like for Blaze Components?

I think it is pretty good. But I have not computed any coverage. If you explain how to do that with Tiny test I would be glad to do so. See: https://github.com/meteor/meteor/issues/4518

ghost commented 9 years ago

@stubailo If you or someone else want to get the gist of the value that Blaze Components provide, http://components.meteor.com/ is the resource to get that. I just went through it again and it explains the value of the composition and inheritance features very well and also shows how Blaze currently comes short with this concerns.

I currently use https://atmospherejs.com/aldeed/template-extension to get the composition feature. But I think Blaze Components has done a better job for the overall picture of components and it is also great to use it with ES2015.

stubailo commented 9 years ago

@mitar what does a BC analog for @Sanjo code sample above look like? https://github.com/meteor/guide/issues/8#issuecomment-146110276

Like, if I wasn't interested in reusing logic between templates at all (which is what the feature set seems to focus on) what benefits do I get from BC?

mitar commented 9 years ago

@mitar Could you provide the equivalent of the code in #8 (comment) for when you write it with Blaze Components? Then we have a one to one comparison.

As I mentioned, there are two issues here. One is boilerplate and structure. The other is that you want to be able to reuse and extend components. The https://github.com/meteor/guide/issues/8#issuecomment-146110276 does not help much with making this template extendable.

If I leave the example verbose (using reactive-field package as well):

class myComponent extends BlazeComponent {
  onCreated() {
    this.validateArguments();
    this.defineState();
  }

  validateArguments() {
    check(this.data(), {
      initialValue: String
    });
  }

  defineState() {
    this.myValue = new ReactiveField(this.data().initialValue);
  }

  doSomething(newValue) {
    this.myValue(newValue);
  }

  events() {
    return [{
      'click .my-button': this.onClick
    }];
  }

  onClick(event) {
    const newValue = this.$('.my-input').val();
    this.doSomething(newValue);
  }
}

myComponent.register('myComponent');

Otherwise I would just do:

class myComponent extends BlazeComponent {
  onCreated() {
    check(this.data(), {
      initialValue: String
    });
    this.myValue = new ReactiveField(this.data().initialValue);
  }

  events() {
    return [{
      'click .my-button': this.onClick
    }];
  }

  onClick(event) {
    const newValue = this.$('.my-input').val();
    this.myValue(newValue);
  }
}

myComponent.register('myComponent');
stubailo commented 9 years ago

@mitar correct me if I'm wrong; I think we are running into the debate between composition and inheritance?

It seems like Blaze Components is squarely in the "inheritance" side of reusability - build a common base component and then extend it with custom functionality by overriding some helpers, or adding some different HTML, or similar. Correct?

mitar commented 9 years ago

I think we are running into the debate between composition and inheritance?

No we are not. :-)

We are in reusability vs. no reusability debate. :-)

Blaze Components provide both ways:

Please read the tutorial: http://components.meteor.com/ Do the homework and I think then the debate will be much more constructive.

mitar commented 9 years ago

(You can switch the tutorial to JavaScript, if you prefer.)

stubailo commented 9 years ago

I did read it.

Refactor the template to {{> include}} another template, or create a {{#block}} helper that provides the functionality.

This is what I would do after reaching the first question.

stubailo commented 9 years ago

The Beauty Components allow us to create new components that share code simply through inheritance.

Also encapsulates what I'm saying.

mitar commented 9 years ago

The Cooperation To address this issue Blaze Components provide mixins with composition over inheritance in mind.

stubailo commented 9 years ago

Yep - I guess maybe we can just only document the mixin approach and not the inheritance one.

But even with mixins, you don't quite have full reusability:

class AutoSelectInputMixin extends BlazeComponent
  events: -> [
    'click input': @onClick
  ]

  onClick: (event) ->
    $(event.target).select()

Now your mixin is coupled to a certain DOM structure, no? The thing that worries me about this approach is that you can easily end up with hidden dependencies between components that look like they are decoupled.

Perhaps it's just a matter of changing the docs to avoid such gotchas?

stubailo commented 9 years ago

Like my question is, how is the above code sample better than a standalone Blaze component like {{> AutoSelectInput }}, where both the select login and the input element itself are encapsulated?

stubailo commented 9 years ago

One sec, let me compose an example that has all three behaviors, AutoSelect, Persistent, and RealTime.

stubailo commented 9 years ago

Ah, OK after some messing around in an editor here are the issues for me:

  1. The selector for the input element should be able to be passed in as an argument to the mixin somehow. Maybe this means you need to include the mixin, and then in onCreate you say @registerAutoSelectInput 'input'
  2. Does the onKeyUp method in RealTimeInputMixin get added to SmartInputComponent? In that case, it shouldn't be a method, because it needlessly adds a method that you will never use that takes up a name in the namespace of possible methods

I guess really what I'm getting at is that it would be good to make it more clear where certain things are wired together just by looking at the component, rather than having to read all of the code of the mixins, the template, and the component.

ghost commented 9 years ago

The selector for the input element should be able to be passed in as an argument to the mixin somehow. Maybe this means you need to include the mixin, and then in onCreate you say @registerAutoSelectInput 'input'

I think behavioral component mixins should not do anything with the DOM of the consuming component at all. The behavioral component mixin can provide methods that the consuming component must call in the right event handlers.

Does the onKeyUp method in RealTimeInputMixin get added to SmartInputComponent? In that case, it shouldn't be a method, because it needlessly adds a method that you will never use that takes up a name in the namespace of possible methods

I think mixins are useful when you want to extend multiple components with some common behavior. In this case the behavior is wanted, that all methods are available on the consumer component. After all that's how mixins work.

There is another kind of composition where you use other components in you template to create something bigger. For example a form component uses input and button components in its template. In this case the used components (inputs and buttons) provide a public API (arguments and methods) that the form can use to communicate with its children.

Both forms of composition have their use cases.

stubailo commented 9 years ago

@Sanjo based on your comment, I am guessing you would agree that the examples given on the Blaze component website aren't optimal in their current form? I think the concepts behind the library are great, but I think the usage examples could be more principled.

mitar commented 9 years ago

OK, before we continue with this discussion, I would like to have a correct frame of mind for it. Because I am getting a bit confused what exactly are we discussing. I though we are discussing which community project are providing reusable components to present them in the guide. This is at least the title of this issue.

So, what is the mindset for which you are asking those questions:

If it is the first, then yes, we can and should have a discussion about which features you would want to put into the core and which not. To better understand design decisions and see if maybe only a part of the package should go into the core.

If the second, then I do not see much benefit from the discussion of the type "it is more powerful than I would need and I do not see why some features are needed". So yes, Blaze Components have more then just fixing the boilerplate issues. They are trying to address reusability. Now, you might prefer some other ways to make things reusable and extendable, but this is then the question of taste. We could discuss this through Blaze Components tickets and see if we can improve them by adding or removing stuff. If you find mistakes or unclear stuff in tutorial, also, please open a ticket. So yes, Blaze Components are definitely not perfect, but they are made and are solving concrete issues while providing some extra stuff to facilitate reusing and extending which probably can be verified only through time.

If it is the third, then yes, we could have a discussion about that as well, but I thought that the idea is to put into the guide existing community projects, not building new. But maybe that can be a good side effect of a guide. To map the Meteor ecosystem and see where are there already good packages and where are blank holes to fill them, and then maybe invite the community to fill them. Maybe even provide some guidance by the MDG.

mitar commented 9 years ago

Yep - I guess maybe we can just only document the mixin approach and not the inheritance one.

What's wrong with the inheritance one? In practice I think that inheritance one is used more often. Many common patterns nicely map to it.

I do not understand what is wrong documenting both approaches? Why hiding a powerful approach just because it is too powerful for some set of use cases? You do not have to use any of those. You do not have to use inheritance, you do not have to use mixins. You can just use it to fix the issue of this in helpers and event handlers.

But, if you want, if your project merits it, you can also use other features, create a hierarchy of components.

Now your mixin is coupled to a certain DOM structure, no? The thing that worries me about this approach is that you can easily end up with hidden dependencies between components that look like they are decoupled.

Sure. Any sophisticated system can lead you to hang yourself. We can again discuss the question of should the system try to prevent this or should this be something you should be doing.

Also, you are reading the tutorial and stop immediately when you see an issue. You should continue reading. Because the approach in tutorial is that we present a problem, show you one solution, then explain why is not the best one, make an improvement, then again look at it, make another improvement and so on.

You look like that smart children in a classroom when teacher starts with the first lecture and wants to make it simple and he goes "oh, this will not work in all cases". Yes. That is the point. It is a simple example. We will take the complicated case in ten lectures. Patience. :-)

Perhaps it's just a matter of changing the docs to avoid such gotchas?

You mean Blaze Components tutorial? Yes, sure, more examples we would have, more literature all around, better it will be. Feel free to open a ticket proposing what you would improve, or make a pull request. The issue with introductory tutorial is always how to make a simple but meaningful examples without throwing too much stuff at the reader. Suggestions are more then welcome. Probably there have been much more examples, and probably with slower pace. At the tutorial we wanted to show many features as soon as possible, but that can overwhelm. Also, more example to show good patterns and practices would also be good.

But, then, this is also one thing we might not want to impose to much. How I see it is like, Blaze Components provide you with this system, with these features, go out there and build something beautiful, find your own patterns, and then come back and show us what you build. I didn't want to force my own ideas how all those features should be used. I have them, but you should also be able to use them differently.

mitar commented 9 years ago

I am getting the feeling that we are discussing now if the Blaze Components tutorial is good and what it could be improved there? I am not sure if this is the right place for that. Feel free to open a ticket in Blaze Components repository about that.

The selector for the input element should be able to be passed in as an argument to the mixin somehow. Maybe this means you need to include the mixin, and then in onCreate you say @registerAutoSelectInput 'input'

Yes, you can do that. See the example in the tutorial:

mixins: -> [
  AutoSelectInputMixin, RealTimeInputMixin,
  CancelableInputMixin, FormFieldMixin,
  new StorageMixin Values, 'value', => @data().id
]

See, the last mixin is getting arguments. Why we didn't do it for all mixins? To show possible different ways to do it.

Does the onKeyUp method in RealTimeInputMixin get added to SmartInputComponent? In that case, it shouldn't be a method, because it needlessly adds a method that you will never use that takes up a name in the namespace of possible methods

No, namespace of mixins is separate from namespace of components. Each mixin instance is its own object.

@stubailo: I would suggest that you write your own tutorial for Blaze Components and I would be glad to link to it from the Blaze Components README. I think more tutorials for different learning styles are there, better it will be.

mitar commented 9 years ago

I guess really what I'm getting at is that it would be good to make it more clear where certain things are wired together just by looking at the component, rather than having to read all of the code of the mixins, the template, and the component.

Yes, that is why that section is called "extreme decomposition" and has this comment:

If this is good or bad depends, but as an exercise let's refactor this example once more.

So tutorial is showcasing features. I do not think it should be seen as a source of best practices. Tutorial is a set of exercises to get you to think in the Blaze Components world and that you know what exists.

So yes, any form of abstractions and decomposition can make code less easy to understands. A good trade-off is important then to achieve. I must admit that in practice for many my components I have not yet had to use mixins and simple inheritance was more than enough. But I used them few times and it makes things pretty in those cases.

In some way, I am hopping for the community to surprise us on innovative ways to decompose and combine components. I just want for them to have tools to do so.

mitar commented 9 years ago

Both forms of composition have their use cases.

And both are supported by Blaze Components. :-)

stubailo commented 9 years ago

@mitar, I really appreciate you taking the time to write all of this to help us understand your Blaze Components package. I wasn't trying to criticize at any point, and I'm sorry if I came off as needlessly negative about the tutorial or anything else - I think it's just my personal style to poke at things until I feel like I understand everything.

My goal has been to understand your ideas and concepts behind the surface of the package you built. As you say, the tutorial and API support many different use cases (which is great), and I wanted to understand your personal approach to designing this stuff. This thread now contains a lot of very relevant information and thoughts about component design in general, and will be a very important resource going forward.

mitar commented 9 years ago

No problem. :-) I love feedback. But I also love replying to feedback. :-) It was just unclear sometimes if you are commenting on the design, pattern, or concrete line of a example. (So are you using example to comment on a broader design question behind it, or on that concrete example.)

But the issue is that I really think it is important to differentiate between the design of a package (which is documented through reference) and the patterns I personally think you should be using for Blaze Components.

I tried really hard to keep possibilities open for how Blaze Components are used. I would like in fact more that people read the reference and then start asking themselves the question, OK, and how to use that now and come up with their own patterns. Because then we can have a diversity of approaches. And different ones will work for different people differently. Instead of proposing the "standard" way of using them. Maybe this leaves people too lost in possibilities, though.

For example, I see Blaze Components as minimal core which gives you ways to extend it through inheritance and composition.

So then I would like to see people to build upon it their own base classes which wrap some other approach on top. For example, MVVP approach could be then build on top (I am personally probably not gonna use it). Or for intra-component communication, somebody could extend base class with event system (but I prefer a more reactive approach). Somebody might create a tree of various form-control components. But somebody else might do it as block helpers instead.

I do not know. And I do not really want to tell which way is the correct way.

So the design behind the Blaze Components is really:

I think this is it.

BTW, if you would like, there are some slight changes to Meteor which might help improve Blaze Components a bit. If MDG would be willing to consider them, that would be great.

SachaG commented 9 years ago

There's a lot of information in this thread that I haven't had time to read yet, so sorry if this is not relevant. But here are a few random thoughts about components.

Extra Callbacks

Here's a package that implements examples of the extra callback idea Tom was talking about. In this case, onSubscribed (triggers when Template.subscriptionsReady returns true) and onDataChanged (triggers whenever the data context changes).

Avoiding "Naked" Data Contexts

Something that's been causing problems for me lately is putting the component's data on this. For example, if I have a component that displays a post, it would naively make sense to pass the post object to the template and do:

<h2>{{title}}</h2>

The reason this is a problem is that at some point I might have some meta-property that affects how the component is displayed (like componentClass). I now have no choice but to add componentClass to my post object, which pollutes my data (not to mention, might lead to naming conflicts). So a better pattern is to pass:

{
componentClass: "foo",
item: post
}

And do:

<h2 class="{{componentClass}}">{{item.title}}</h2>

Template Extension

Something that's been super-useful when working with components is @aldeed's template extension package.

For example, if I let users customize some part of the component with their own custom templates, I can then use Template.foo2.inheritsHelpersFrom("foo"); to copy over the helpers from the original component template to the new custom template.

mitar commented 9 years ago

Oh, I forgot about one other interesting question. Should Spacebars.call be modified to always return null?

https://github.com/peerlibrary/meteor-blaze-components/issues/21 https://github.com/meteor/meteor/issues/4230 https://github.com/meteor/meteor/issues/4232

mitar commented 9 years ago

@stubailo, I am not sure if this is the best place to discuss this (please point me to it), but I would like to understand your concerns with Blaze and see if they can be addressed in Blaze Components (if they are not already). So I think only these two are potentially not yet addressed by Blaze Components:

The data context being passed from outside of the template means the functionality depends on the outside environment in a poorly-defined way

I really have issues understanding what exactly is problem with data contexts. Maybe I developed my own coping patterns so that data context works for me very well.

So is it the problem that:

Is it something else?

I do not see the first really as a problem (hm, does Spacebars support passing an empty data context when including a template?, to force an empty one over the current one)? This is for me simply a sugar.

The second could be made by doing some checks like mentioned above, or some common way to document those? Textually or through the code?

The third is easy to solve with [computed-field](https://github.com/peerlibrary/meteor-computed-field0 package and could maybe be combined with checks to provide automatic per-field values. But for me this is something which is premature optimization in a general case and I think it is better to do such optimizations once you see the problem. See comment here for a similar approach.

The last I do by first traversing the components (or template tree, using template-extension package) to find the component I want and then I use their data context. So even as things are moved around, until my search function is semantically specified (often something like searching for a component which has this field) things work. For me this is also a pattern which allows more sophisticated inter-component communication and accessing the parent data context is just a special case of it. But yes, then we also have to find a good way to document such access. But that could also be just a coding pattern to do that.

The way event maps work means they also catch events from child templates, which is something you might or might not want

So that I think can be addressed with a very small extension to Blaze Components which can be then be a standalone new base class, or just a mixin: https://github.com/peerlibrary/meteor-blaze-components/issues/88 So one can choose one or another approach very easily.

tmeasday commented 9 years ago

This ticket has been dominated by a conversation about Blaze Components, so I've renamed it.

There is a second ticket https://github.com/meteor/guide/issues/42 in which I focus on the core topics to be discussed. I'm doing it in an agnostic way (assume that we could do it both w/ and w/o BC).

Please let me know if you think I missed anything

tmeasday commented 9 years ago

To be explicit here, you may have noticed the current version of the Blaze outline does not use BC.

The plan that @stubailo came to is to write a first version that uses vanilla Blaze w/ a lot of boilerplate & best-practice in the style of @Sanjo's comments above.

That document can then be a central point of decision making about where Blaze should move next (not to say I don't expect any decision to be made before the document is done! I sincerely hope not). I think with that document in front of us and components.meteor.com or the BC docs as a counter point.. well you can use your imagination.

mitar commented 9 years ago

Hm, I see. But I think that the boilerplate solutions are just half way. So with Blaze Components this is what our progression was. We first make it so that things are defined in more sane ways (template instance as this and consistent behavior in template helpers and event handlers) and that you can easily access state on template instances. But that is just half way. Because then there are even more important questions: how you make things reusable and how you make things so that people can make packages. This is why then we added stuff like splitting templates (view) from components (logic). So that components can reuse the same template, and that component can stay the same, but you just replace the template below it. Use cases are things like account forms. Where you have a default template, but sometimes you just want to change a bit of text to allow there. So you could replace a template with your own, while all the logic would stay from the original package. Now, packages like useraccounts are a real mess when you want to make every part extensible. And there is no clean common way. Similarly with mixins, which can add some behaviors (like permission checks, form validations, form interactions, animations).

So yea, boilerplate is like Blaze 1.1. Fix of Blaze 1 and make improved decisions based on experience. But to really create an ecosystem of reusable components and behaviors something like additional things we added to Blaze Components are necessary. For example, boilerplate solution does not tell you how to make then a reusable template instance. Which you could extend and add one more event handler on (while keeping the original template intact). You can still use then hacky template-extensions.

So I think the guide would benefit from at least one small chapter or paragraph outlining this features of Blaze Components. Something like "do you want to move it even further in encapsulation and code reuse"? And yes, the main question we should also answer is: how can people write template instances they can put into packages so that others can reuse? So maybe we can also do just a short section with like, if you want to use it only for yourself, then this boilerplate is good for you, but if you want to make reusable templates, then consider this section in packaging of reusable components, and then we describe Blaze Components more there.

mitar commented 9 years ago

(That was also the previous title of this ticket. How to make reusable components. Boilerplate is how to structure your own templates. But how to make them reusable? I think we should encourage community around sharing components.)

tmeasday commented 9 years ago

It's a good point that you make. I'd like to give some more thought to this reasoning behind BCs being a separate construct.

mitar commented 8 years ago

So, what is the status of this? I checked current published guide and it is really funny how much of the stuff there is just workarounds and long explanations for things which Blaze Components are already solving. It looks so much extra work needed to be done by developers.

BTW, @tmeasday, @stubailo, check out the new template-level event handlers support: https://github.com/peerlibrary/meteor-blaze-components#handling-events