-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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 support for typed arrays #5905
Conversation
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.
Thanks @kurkle.
helpers.isArray
is used a lot even outside this project, so we need to be sure that returning true
for all arrays (basic and typed) will not break existing code. Is there any use case where we don't want to handle both the same way?
0607db9
to
38fd7bd
Compare
There is one place, where I get the idea this could break outside code
Here, if So if someone tested with helpers.isArray and based on that tried to |
I didn't know that typed arrays didn't support push/pop. I don't think that's an issue because it would fail only if this library or external plugins try to alter the size of a typed array after checking If that's the only difference, I think it's safe to merge (though, not fully confident). @etimberg @nagix do you think of use cases that this PR would break? |
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.
Thanks @kurkle
@@ -1,4 +1,8 @@ | |||
parserOptions: | |||
ecmaVersion: 5 # don't rely on default, since its changed by env: es6 |
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.
It should be fine if our tests use es6 features, we only don't want our sources to use es6 feature because we don't want to integrate babel in v2.
https://codepen.io/kurkle/pen/jXEwYe
Fixes #4920