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

Array fixes #925

Conversation

romainmenke
Copy link
Collaborator

@romainmenke romainmenke commented Dec 4, 2020

Fixes for Array

IE8

  • check for Symbol in _ESAbstract/ToInteger [1]
  • Array.sort stable fix must check for ownProperty [2]

IE8 does not get the Symbol.prototype.description polyfill.

IE9-11

  • Array.prototype.@@iterator fail will be fixed with a Symbol refactor [3]
  • Array.prototype.entries fail will be fixed with a Symbol refactor [3]
  • Array.prototype.keys fail will be fixed with a Symbol refactor [3]

Other browsers

did some spot tests for older chrome/firefox/safari versions

These show the same results as IE9-11.


Notes :

(1) Array.prototype.flat uses ToInteger which lacked a check for Symbol.
This test is skipped when self.Symbol doesn't exist.

(2) Array.prototype.sort has a fix for stable sorting.
This uses a for .. in iteration which didn't have a check for ownProperty.

On master :

for (var a in that) {
    console.log(a);
    if (that[a].item !== this[a]) {
        this[a] = that[a].item;
    }
}

Screenshot 2020-12-04 at 19 51 37

This is not ideal, but didn't throw before.
When all polyfills are loaded and Array.prototype is fully polyfilled it does throw when handing one of the inherited properties.

(3) Symbol.iterator is undefined because Symbol.prototype.description uses a wrapper to intercept the arguments.
This wrapper does not restore the static methods on Symbol.

Symbol.iterator being missing breaks some Array methods which rely on iterators.
This will not be fixed in this branch.

@romainmenke romainmenke requested a review from a team as a code owner December 4, 2020 16:48
@origamiserviceuser origamiserviceuser added this to incoming in Origami ✨ Dec 4, 2020
@github-actions github-actions bot added the library Relates to an Origami library label Dec 4, 2020
@romainmenke romainmenke marked this pull request as draft December 4, 2020 16:51
@romainmenke romainmenke marked this pull request as ready for review December 4, 2020 19:05
@JakeChampion JakeChampion merged commit c6c1cfb into JakeChampion:powowow Dec 9, 2020
Origami ✨ automation moved this from incoming to complete Dec 9, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2021
@robertboulton robertboulton removed this from Done in Origami ✨ Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
library Relates to an Origami library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants