-
Notifications
You must be signed in to change notification settings - Fork 93
refactor: drop string-length
dependency and replace with native solution
#6779
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
Conversation
…ution Signed-off-by: Ferdinand Thiessen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and INTL is widely supported by now.
@@ -471,7 +474,8 @@ export default { | |||
if (this.isEmptyValue || !this.maxlength) { | |||
return false | |||
} | |||
return stringLength(this.localValue) > this.maxlength | |||
const length = [...this.segmenter.segment(this.localValue)].length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, I love JavaScript's horrific iterator abstraction ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks clean to me 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also would like if they returned an proper array like but on the otherside browser implementation could be probably faster with a generator like it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it makes sense. I was referring to the incomplete API, for example, a <iterable>.count()
function like in Rust etc. so that we don't have to copy memory all the time when working with iterators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS: But it will get better very soon, now that an important proposal was merged.
☑️ Resolves
Remove useless dependencies where we can just use native ES alternatives.
🏁 Checklist
stable8
for maintained Vue 2 version or not applicable