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

fix(uniq): update example to cover Internet Explorer #158

Closed
wants to merge 3 commits into from

Conversation

yarnball
Copy link

I've found an issue with _.uniq and Internet Explorer (IE).

IE does not support new Set(iterable)

See screenshot from mdn below:
screen shot 2018-11-30 at 11 30 03 am

So, I've updated the example to be IE compatible.

[...new Set(array)];

to...

 array.filter((v, i, a) => a.indexOf(v) === i)

@yarnball yarnball changed the title fix(uniq): update example to cover ie11 fix(uniq): update example to cover Internet Explorer Nov 30, 2018
@coveralls
Copy link

coveralls commented Nov 30, 2018

Coverage Status

Coverage remained the same at 91.667% when pulling 76ca7df on yarnball:fix-uniq-ie into 6f008f5 on you-dont-need:master.

Copy link
Member

@stevemao stevemao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create a new example instead of changing existing one?

README.md Outdated
* Please note that, the examples used below are just showing you the native alternative of performing certain tasks. For some of the functions, Lodash provides you more options than native built-ins. This list is not a 1:1 comparison.

* Please send a PR if you want to add or modify the code. No need to open an issue unless it's something big and you want to discuss.
**Please note that, the examples used below are just showing you the native alternative of performing certain tasks. For some of the functions, Lodash provides you more options than native built-ins. This list is not a 1:1 comparison.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you change these?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yarnball please check this review comments 😄

@@ -198,7 +198,7 @@
},
"uniq": {
"compatible": true,
"alternative": "[... new Set(arr)]",
"alternative": "Array.prototype.filter((v, i, a) => a.indexOf(v) === i)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? this guide is not only for IE. Supporting IE is nice to have

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still hasn't actually been changed after review requested

README.md Outdated
console.log(result)
// output: [1, 2, 4, 3]
// output: [1, 2, 3, 4]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the original. Add a new one and comment "For IE"

README.md Outdated
* Please note that, the examples used below are just showing you the native alternative of performing certain tasks. For some of the functions, Lodash provides you more options than native built-ins. This list is not a 1:1 comparison.

* Please send a PR if you want to add or modify the code. No need to open an issue unless it's something big and you want to discuss.
**Please note that, the examples used below are just showing you the native alternative of performing certain tasks. For some of the functions, Lodash provides you more options than native built-ins. This list is not a 1:1 comparison.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yarnball please check this review comments 😄

@@ -198,7 +198,7 @@
},
"uniq": {
"compatible": true,
"alternative": "[... new Set(arr)]",
"alternative": "Array.prototype.filter((v, i, a) => a.indexOf(v) === i)",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still hasn't actually been changed after review requested

@@ -511,11 +611,11 @@ Gets the first element or all but the first element.
// output [2, 3]
```

#### Browser Support
#### Browser Support for Spread in array literals

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have you updated these?


**[⬆ back to top](#quick-links)**

### _.intersection
Returns an array that is the intersection of all the arrays. Each value in the result is present in each of the arrays.
## Collection*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to add new methods, do them in a new request.

We try to keep it to one change of method per PR

@yarnball yarnball closed this Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants