Skip to content
This repository has been archived by the owner on Jun 27, 2018. It is now read-only.

#57 Fix for..in bug, add tests to guard against regressions. #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RichardBradley
Copy link
Contributor

This fixes #57, and adds tests which a) demonstrate why this is required and b) should guard against regressions.

@RichardBradley
Copy link
Contributor Author

(This is rather ugly, but I think it is necessary. If we had "angular" in scope then a most of these could be fixed with "angular.foreach", but admin-config doesn't currently have angular as a dependency.)

The former causes "ReferenceError: Symbol is not defined" in ng-admin.
@RichardBradley
Copy link
Contributor Author

I found that using "for .. of" breaks ng-admin with "ReferenceError: Symbol is not defined" errors, so I've removed that from this PR.

@RichardBradley
Copy link
Contributor Author

Any thoughts on this?
This is necessary for me to use ng-admin at the same time as polyfills like Array.find

@RichardBradley
Copy link
Contributor Author

Any update on this one?
I have added some tests which fail before this change and pass after it.

Do you not like the "for..in" syntax? It is ugly, but I think it is necessary, as demonstrated above. If you don't want to make this change, I understand.

@djhi
Copy link
Contributor

djhi commented Aug 25, 2017

Sorry about that. The PR looks fine to me expect it needs to be rebased.

@djhi djhi requested a review from fzaninotto August 25, 2017 15:09
@RichardBradley
Copy link
Contributor Author

From my point of view, this is ready to merge.

I won't rebase it myself, as I am not confident that it would get merged were I to spend that time. It should be only a few minutes work to rebase & fix the conflicts if anyone with write permission wanted to do so.

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

Successfully merging this pull request may close these issues.

Iterating over array entries using "for .. in" breaks if Array polyfills are used
2 participants