Skip to content

Simplify toString #4441

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

Merged
merged 3 commits into from
Aug 25, 2019
Merged

Simplify toString #4441

merged 3 commits into from
Aug 25, 2019

Conversation

blikblum
Copy link
Contributor

It uses native array map instead of custom one and assumes Symbol.prototype.toString exists

@@ -36,10 +32,10 @@ function toString(value) {
}
if (Array.isArray(value)) {
// Recursively convert values (susceptible to call stack limits).
return `${map(value, (other) => other == null ? other : toString(other))}`
return `${value.map((other) => other == null ? other : toString(other))}`

Choose a reason for hiding this comment

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

i feel () for map parameters is redundant here, can we not use

return ${value.map( other => other == null ? other : toString(other))} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its the project style. See #4418 (comment)

@jdalton
Copy link
Member

jdalton commented Aug 25, 2019

Thank you @blikblum!

@jdalton jdalton merged commit abb54cc into lodash:master Aug 25, 2019
@blikblum blikblum deleted the simplify-tostring branch August 25, 2019 22:59
@jdalton jdalton added issue bankruptcy Closing the issue/PR to start fresh and removed issue bankruptcy Closing the issue/PR to start fresh labels Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants