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

Fix #7404: skip v-model & value binding collision check with dynamic type binding #7406

Merged
merged 2 commits into from
Mar 7, 2018

Conversation

Justineo
Copy link
Member

@Justineo Justineo commented Jan 8, 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:

None.

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:

None.

@Justineo Justineo changed the title Fix #7404 Fix #7404: skip v-model & value binding collision check with dynamic type binding Jan 10, 2018
@HerringtonDarkholme
Copy link
Member

I wonder how generated code handle duplicate properties for code like <input :type="type" :value="v" v-model="m">.

Generated code will have a conditional branch for type = "text", in that branch, I wonder if Vue will generate domProps: { value: ..., value: ...}. As the original warning is to avoid duplicate properties error in IE11, lifting the constraint alone will make the error regress.

@Justineo
Copy link
Member Author

Generated code will have a conditional branch for type = "text"

Yes I'm aware of this. It may become this question: are we gonna emit a warning message for something might cause error but in we cannot be sure about that during the compiling stage? When the user handles the data correctly, it will work as expected. Or in this case, at least we should warn that v-model and :value will conflict for some input types so users should be careful about the actual type they provide.

@HerringtonDarkholme
Copy link
Member

are we gonna emit a warning ... during the compiling stage?

In this case it might be too noisy for legitimate use. Warning after this pull request is good enough IMHO. However, a patch is still needed to handle code generation, like suppress :value emission when both type is dynamic and v-model is set.

If warning is really desirable, generating a runtime warning in type="text" branch is more pleasant in terms of DX. But it should be fine without it.

@yyx990803 yyx990803 merged commit 1c0b4af into vuejs:dev Mar 7, 2018
@Justineo Justineo deleted the fix-7404 branch March 8, 2018 03:22
f2009 pushed a commit to f2009/vue that referenced this pull request Jan 25, 2019
aJean pushed a commit to aJean/vue that referenced this pull request Aug 19, 2020
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.

4 participants