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

Aurelia polyfills are enumerable #12

Closed
jods4 opened this issue Mar 8, 2016 · 5 comments
Closed

Aurelia polyfills are enumerable #12

jods4 opened this issue Mar 8, 2016 · 5 comments
Assignees
Labels

Comments

@jods4
Copy link
Contributor

jods4 commented Mar 8, 2016

When you simply assign the prototype, like so:
Array.prototype.includes = function...
You introduce a new enumerable member for all arrays.

So if someone had written code like this:

for (var i in myArray) {
  var v = myArray[i];
  // do whatever with v
}

It's very likely to now break, because "includes" will be an iteration of i.

This is a regression wrt previous releases that used corejs. I didn't check but I guess that CoreJs defines a non-enumerable property for its polyfills.

@EisenbergEffect
Copy link
Contributor

Well, you shouldn't use a for/in loop with an array ;) But, we'll get it fixed.

@EisenbergEffect EisenbergEffect self-assigned this Mar 8, 2016
@jods4
Copy link
Contributor Author

jods4 commented Mar 8, 2016

I was sure you would say that. 😉

I don't approve but I'm not the only committer to our code base...
FWIW, this was considered common enough that recently the TS team has taken special steps to support this pattern...

@EisenbergEffect
Copy link
Contributor

We'll fix it. I'm looking at it now. No problem :)

@jods4
Copy link
Contributor Author

jods4 commented Mar 8, 2016

Reading my comment above I realize it was ambiguous... For the record I don't approve the practice of for..in over arrays, not your comment ;)

Just sayin': a quick glance over the code shows that string.js has the same problem, although it's certainly a lot less likely to trigger issues. I know of noone who does for (var i in "troll").

@EisenbergEffect
Copy link
Contributor

Yeah, I understood your meaning :) I fixed the array issues but wasn't sure if it was worth it to fix the string issues. It's not that big a deal though. I could do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants