Skip to content

Conversation

@karevn
Copy link

@karevn karevn commented Nov 16, 2015

Continuation of this work: #1441

yyx990803 added a commit that referenced this pull request Nov 16, 2015
Reverse filters support for v1.0
@yyx990803 yyx990803 merged commit 904b905 into vuejs:dev Nov 16, 2015
@yyx990803
Copy link
Member

👍 thanks!

@yyx990803
Copy link
Member

Unfortunately, had to revert this because this actually neglects a use case where the user attempts to use v-model on a v-for alias with a read-only filter, e.g. orderBy. This is in fact pretty common and is what the original warning is trying to prevent.

With this PR, such use case no longer receives a warning and actually writes the value back to the original array at the wrong index.

@karevn
Copy link
Author

karevn commented Nov 16, 2015

Ok. Any ideas for enhancements?
16 нояб. 2015 г. 21:50 пользователь "Evan You" notifications@github.com
написал:

Unfortunately, had to revert this because this actually neglects a use
case where the user attempts to use v-model on a v-for alias with a
read-only filter, e.g. orderBy. This is in fact pretty common and is what
the original warning is trying to prevent.

With this PR, such use case no longer receives a warning and actually
writes the value back to the original array at the wrong index.


Reply to this email directly or view it on GitHub
#1818 (comment).

@yyx990803
Copy link
Member

I tried working on top of your PR but haven't had a clean solution yet. Will take a deeper look later this week.

@karevn
Copy link
Author

karevn commented Nov 16, 2015

I think we should figure out use cases first. :)
16 нояб. 2015 г. 22:05 пользователь "Evan You" notifications@github.com
написал:

I tried working on top of your PR but haven't had a clean solution yet.
Will take a deeper look later this week.


Reply to this email directly or view it on GitHub
#1818 (comment).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants