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(runtime-dom): v-model breaks minlength validation #1937

Merged
merged 7 commits into from
Aug 25, 2020
Merged

fix(runtime-dom): v-model breaks minlength validation #1937

merged 7 commits into from
Aug 25, 2020

Conversation

unbyte
Copy link
Contributor

@unbyte unbyte commented Aug 23, 2020

more detail: #1935 (comment)

close #1935

@unbyte unbyte changed the title fix(runtime-dom): v-model breaks HTML validation fix(runtime-dom): v-model breaks minlength validation Aug 23, 2020
packages/runtime-dom/src/directives/vModel.ts Outdated Show resolved Hide resolved
packages/runtime-dom/src/modules/props.ts Outdated Show resolved Hide resolved
@dsonet
Copy link
Contributor

dsonet commented Aug 24, 2020

Why not just compare the 2 values? The less involved the better performance.

@unbyte
Copy link
Contributor Author

unbyte commented Aug 25, 2020

Why not just compare the 2 values? The less involved the better performance.

some biased performance tests misled me. it's my fault. 😭

@unbyte unbyte requested a review from yyx990803 August 25, 2020 03:07
@unbyte
Copy link
Contributor Author

unbyte commented Aug 25, 2020

🤐I have not processed a changes requested review, I am a bit at a loss, may have hit the inappropriate button

next !== prev ||
(hostForcePatchProp && hostForcePatchProp(el, key))
(hostForcePatchProp && hostForcePatchProp(el, key)) ||
next !== prev
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you change this order? Typically a compare operation would be lighter than a method invoke. It's reasonable to have the comparison ahead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes you are right. I think I should learn more about javascript optimization.
https://jsben.ch/dSVTE

@yyx990803 yyx990803 merged commit 1d55454 into vuejs:master Aug 25, 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.

v-model breaks HTML's minlength attribute validation
3 participants