Skip to content

with directive #5269

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

Closed
rhyek opened this issue Mar 24, 2017 · 30 comments
Closed

with directive #5269

rhyek opened this issue Mar 24, 2017 · 30 comments

Comments

@rhyek
Copy link

rhyek commented Mar 24, 2017

What problem does this feature solve?

This would provide something akin to a with block. It would be useful to me especially in cases like the one shown here. During iterations where your context is essentially a superset of the current component's, you can't have a computed property that defines person since it would be different for every meeting.

There are, of course, ways around this. The most straightforward would be to set person on the meeting object during creation. The issue is that creating the object might be done in different ways (locally, initial data load, server broadcast, etc) and you would need to handle all those cases, preferably in a DRY manner.

It could be made to only work with v-for to avoid this.

What does the proposed API look like?

<template>
  <table>
    <tbody>
      <tr
        v-for="meeting in meetings"
        with="{ person: people.find(person => person.id === meeting.personId) }"
      >
        <td>{{ meeting.date }}</td>
        <td>{{ person.firstName }}</td>
        <td>{{ person.lastName }}</td>
      </tr>
    </tbody>
  </table>
</template>

<script>
export default {
  data() {
    return {
      people: [
        { id: 1, firstName: 'Felix', lastName: 'Hudson' },
        { id: 2, firstName: 'Gaby', lastName: 'Rápalo' }
      ],
      meeting: [
        { date: '2017-01-01', personId: 1 },
        { date: '2017-03-02', personId: 1 },
        { date: '2017-02-07', personId: 2 }
      ]
    }
  }
}
</script>
@nickmessing
Copy link
Member

@rhyek, you could actually do a computed property for that.

@nickmessing
Copy link
Member

Actually I do like the idea, but I would call it v-scope because it adds aditional values in scope that is already created by v-for.

@posva
Copy link
Member

posva commented Mar 24, 2017

It's better to create a component for that and compute there only once the heavy operation you need

@rhyek
Copy link
Author

rhyek commented Mar 24, 2017

@posva this is actually intended for one off computations you won't need anywhere else. It's simpler, quicker, and offers more readability.

For more complex and reusable computations and functionality, I always use the approach you mention.

@posva
Copy link
Member

posva commented Mar 25, 2017

It's simpler, quicker, and offers more readability

It's quicker to add once you know the syntax, but it's one more thing to know and adds another way of doing the same thing. That said, I see the benefits and simplicity of it, I simply think it's better to use a component, even if it's just declared on one component, because if you know that way you know a more powerful way of abstracting things, and will probably understand better how to use components

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017

@posva, for computing one property such as I do in my example, I would probably use @nickmessing's suggestion, currently. If I need more complexity/functionality, I tend to use components for that. I agree with you that components are the most powerful option (in fact, I like them a lot), but for some cases it might feel a bit overkill.

IMO:

  • One or maybe two computations: with or v-scope
  • Two or more: @nickmessing's suggestion
  • More than a few properties, use of any other view model functionality (watchers, methods, etc), own template, etc: Components

@posva
Copy link
Member

posva commented Mar 25, 2017 via email

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Mar 25, 2017

I think that it is not difficult to achieve this issue, but the template syntax check is a problem.

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017

I think with="{ person: people.find(person => person.id === meeting.personId) }" wouldn't really work because what happens if something other than an object is returned, right?

It could instead be something like:

      <tr
        v-for="meeting in meetings"
        v-scope="{ person: people.find(person => person.id === meeting.personId) }"
      >
        <td>{{ meeting.date }}</td>
        <td>{{ scope.person.firstName }}</td>
        <td>{{ scope.person.lastName }}</td>
      </tr>

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017

Another (maybe dumb) example:

<tbody>
  <template
    v-for="(meeting, index) of meetings"
    v-scope="{ class: [index % 2 === 0 ? 'even' : 'odd'] }"
  >
    <tr class="summary" :class="scope.class">
      ...
    </tr>
    <tr class="detail" :class="scope.class">
      ...
    </tr>
  </template>
</tbody>

You get the idea. @posva, you can't use components here because they can only have one root element. 😄

@posva
Copy link
Member

posva commented Mar 25, 2017

Actually, you can, the tbody can be a component

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017 via email

@gebilaoxiong
Copy link
Member

@posva I think that in some cases he is right, everyone wants to write a little code to achieve more features :)

@pbastowski
Copy link

@rhyek
What you are asking for is already available, but under a different disguise:

    p(
      v-for="(meeting, i, person=people.find(person => person.id === meeting.personId)) in meetings",
    )
        | {{ meeting.date }} |
        | {{ person.firstName }} |
        | {{ person.lastName }}

See http://jsbin.com/lelizuc/1/edit?html,js,output

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017

That's kinda funny. It definitely works and I think I actually like this syntax better. This is something that should maybe be mentioned in the guides?

Anyways, thank you @pbastowski.

Feel free to close the issue if so desired.

@rhyek
Copy link
Author

rhyek commented Mar 25, 2017

Eh, apparently the expression in v-for can't be broken into different lines:

      <tr
        v-for="(
          meeting,
          index,
          person=people.find(person => person.id === meeting.personId)
        ) in meetings"
      >

Bummer.

@gebilaoxiong
Copy link
Member

@rhyek I submitted a PR, hoping to help you

@gebilaoxiong
Copy link
Member

@pbastowski In fact the above method does not solve all the situation, I think add for with in template is a better way

@pbastowski
Copy link

@gebilaoxiong My method solves the reported problem. I use this method myself.

So, just wondering, what problems does it not solve that a "with" directive will solve?

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Mar 26, 2017

@pbastowski There are a lot of ways to solve the problem, you also want to be able to maintain a good code, rather than write a line, is not it?

@pbastowski
Copy link

@gebilaoxiong Your statement and the following leading question is so broad that it can apply to any aspect of programming and perhaps even life itself :)

But seriously, I agree with you. A with directive would be a nice addition. Until then, my solution will probably help a few people, who like me, need this functionality now. I hope your PR gets merged.

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Mar 26, 2017

@pbastowski Sorry, i think for a moment, is there a better way?

Actually inside the vue, your method looks just a hack, if meettings is the object then person is always indexed and not the correct value.

https://vuejs.org/v2/api/#v-for

 <div v-for="(val, key, index) in object"></div>

Your method finally generates the code that's it like this:

  function (meeting, index, person = people.find(person => person.id === meeting.personId)) {
    // ...
  }

and also this method only supports one parameter, if there are multiple?

alias is used to store key, value, or index, and the with directive is to add other iterations. From the grammar can also be a good distinction.

My English is not very good, no intention to offend :)

@pbastowski
Copy link

@gebilaoxiong Yes, that is correct. My method only works for iterating arrays, like in the question asked in this issue. Iterating objects would not work at all.

@CodinCat
Copy link
Contributor

I feel it may over complexify the template syntax. Any of computed properties, methods and components can solve these problems easily.

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Mar 27, 2017

@CodinCat As I said, all roads lead to Rome. But we want to choose a good way :)

  • add method findPerson
  <tr v-for="(meeting, index) in meetings">
    <td>{{ meeting.date }}</td>
    <td>{{ findPerson(meeting.personId).firstName }}</td>
    <td>{{ findPerson(meeting.personId).lastName }}</td>
  </tr>

You can imagine, each field needs to be calculated once

  • add computed properties
    can not be solved

  • use component
    can be done,but need to provide a component
    looks a bit twists and turns

So I think the with directive is a good way, it does not need to add new members on vm, nor does it need to provide a component

you can use it like this

  <tr v-for="meeting in meetings"
    with="{ person: people.find(person => person.id === meeting.personId) }">
      <td>{{ meeting.date }}</td>
      <td>{{ person.firstName }}</td>
      <td>{{ person.lastName }}</td>
  </tr>

@Kingwl
Copy link
Member

Kingwl commented Mar 27, 2017

@gebilaoxiong
Add the computed properties of course can solve this

computedMeetings () {
    return this.meetings.map(meeting => Object.assign({}, meeting, {person: this.people.find(person => person.id === meeting.personId)}))
}
<tr v-for="meeting in computedMeetings">
  <td>{{ meeting.date }}</td>
  <td>{{ meeting.person.firstName }}</td>
  <td>{{ meeting.person.lastName }}</td>
</tr>

@gebilaoxiong
Copy link
Member

@Kingwl Well, maybe I'm too horn, I just make a suggestion, use with You do not have to define a computed

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Mar 27, 2017

@rhyek @gebilaoxiong

I'm afraid with way is hardly scalabe.

Consider the trivial example above, it already has been quite verbose to specify I want an attendant in a meeting. Readabilty and conciseness is objective, but at least for me the proposed solution isn't the sweetiest syntax.

A larger component will probably worsen it. For example, you also want to show the attendant's meeting note in the component. More logic is crammed to with directive. The template will soon become unreadable and atrocious to modify. Note: we don't have, sadly, a decent syntax/type checker for expressions in template (correct me if I missed some awesome tool). To reduce the complexity of template one will move the logic to vm/script, which is contrary to the point of this proposal! The predicate in with directive will also become more complex. Consider: you want to display a person in meeting, but only if they is available according to their schedule. The function becomes with="{person: people.find(person => person.id === meeting.personId && person.schedule[meeting.time] === undefined)}", and it will become more complex with projects grow and requirements change.

A larger team will also worsen it. A programmer wants to skip a computed property, so does another programmer. In team work a with directive will, based on my own observation, promote more logic in template because one easywith directive added initially will grows to a huge code blob by different team members. We are always too lazy to add abstraction.

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Mar 27, 2017

@HerringtonDarkholme thx for your reply, I agree very much :)

@rhyek
Copy link
Author

rhyek commented Mar 27, 2017

Thank you all for your interest. I feel I agree with @HerringtonDarkholme and can see the issues this feature might develop for larger teams/projects. Using a computed property that maps my array or @pbastowski's solution are both good ways to provide the functionality I need.
I will close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants