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

Add iterable supports for v-for #8179

Merged
merged 3 commits into from
Dec 26, 2018
Merged

Add iterable supports for v-for #8179

merged 3 commits into from
Dec 26, 2018

Conversation

mymyoux
Copy link
Contributor

@mymyoux mymyoux commented May 12, 2018

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Adds support for iterable object implementing Symbol.iterator
It shouldn't have an impact for existing code because non iterable objects are not impacted.
Vue will have the same behaviour than a for...of expression for iterables. (actual interest for using iterables)
Allows us to pass an iterable directly to a template without giving internal properties or converting it to an array

I wrote 4 new tests. No existing tests needed changes.
All tests passed. I ran tests with PhantomJS (lack of Symbol support), Firefox and Chrome.

@posva
Copy link
Member

posva commented May 12, 2018

note: git conflict with #7171

@mitar
Copy link
Contributor

mitar commented May 14, 2018

Related: #5893

@mitar
Copy link
Contributor

mitar commented May 14, 2018

So I have been using the following for a while now: https://github.com/vuejs/vue/blob/cb062c50cdd21628dcf758fe54c84d17d75cb2ca/src/core/instance/render-helpers/render-list.js

But I thought that in #5893 it is said that this will not go into core for now?

next = iterator.next()
if (!next.done) {
key = i
ret.push(render(next.value, key, i))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just ret.push(render(next.value, ret.length))?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point I can update this

@mitar mitar mentioned this pull request May 14, 2018
12 tasks
@mymyoux
Copy link
Contributor Author

mymyoux commented May 14, 2018

But I thought that in #5893 it is said that this will not go into core for now?

I'm aware of this but I think if you choose to implement [Symbol.iterator] you choose to break old browsers compatibility. This way your iterable object has the same behaviour in Vue than in other parts of your code.
What do you think ?

@mitar
Copy link
Contributor

mitar commented May 14, 2018

I personally like it, I was all for #5893.

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

Successfully merging this pull request may close these issues.

4 participants