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 Iterator supports for v-for #7171

Closed
wants to merge 6 commits into from
Closed

Add Iterator supports for v-for #7171

wants to merge 6 commits into from

Conversation

lyh2668
Copy link

@lyh2668 lyh2668 commented Dec 2, 2017

添加v-for循环对迭代器(Iterator)的支持

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:

添加v-for循环对迭代器的支持
remove semicolon
fixed lint.
The array literal notation [] is preferrable  no-array-constructor
var to let
一开始是在编译后的文件修改的,不敢使用ES6+的语法,比如for of
for (i = 0, l = keys.length; i < l; i++) {
key = keys[i]
ret[i] = render(val[key], key, i)
if (keys.length === 0 && val.toString().indexOf('Iterator') > -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Iterator looks like new to me. Would you like to give me a link of introduction to Iterator? I might be wrong but Google only gave me Generator.

Copy link
Author

Choose a reason for hiding this comment

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

  1. Map.prototype.keys() =>return a new Map iterator object.
  2. Set.prototype.values() => return a new Iterator object containing the values for each element in the given Set, in insertion order.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. I have checked the spec and MapIterator's @@toStringTag symbol returns Map Iterator. The similar for other iterators.

GIven this, I suspect if a toString test is legitimate. Checking Symbol.iterator in proper environment is better, I think.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, the better way is checking Symbol.iterator. At first I thought I can't use the ES6 grammar.

Copy link
Author

Choose a reason for hiding this comment

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

Symbol.toStringTag
A string value used for the default description of an object. Used by Object.prototype.toString().
Symbol

@Gudradain
Copy link

Iterator can be infinite no? Would that be a problem?

@blai
Copy link

blai commented Apr 10, 2018

echo @Gudradain's comment, we need to add a limit to this (maybe with a default value), but the same could go for traditional Array with excessive amount of data (so it's virtually infinite)

@posva posva mentioned this pull request May 12, 2018
13 tasks
@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?

@mitar
Copy link
Contributor

mitar commented May 14, 2018

I think #8179 is a better implementation of this. Better check for iterator support.

@yyx990803
Copy link
Member

Thanks for the PR, closing in favor of #8179 due to better support check and tests.

@yyx990803 yyx990803 closed this Dec 26, 2018
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.

6 participants