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

better string coercion #179

Merged
merged 1 commit into from Sep 24, 2021
Merged

better string coercion #179

merged 1 commit into from Sep 24, 2021

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Sep 23, 2021

For objects that implement both valueOf and toString, we want to favor the toString:

({valueOf: () => 42, toString: () => "foo"}) + "" // returns "42"
`${({valueOf: () => 42, toString: () => "foo"})}` // returns "foo"

This change is particularly critical for APIs such as the proposed Temporal API which will throw an error if you try to concatenate them with a string, even though they implement toString!

Fixes #178.

@mbostock mbostock requested a review from Fil September 23, 2021 22:15
@Fil
Copy link
Collaborator

Fil commented Sep 24, 2021

wow; and the error message about comparisons is quite misleading, since we're not trying to compare.

We'll have to fix that also in all D3 modules I guess?

@mbostock
Copy link
Member Author

We'll have to fix that also in all D3 modules I guess?

Yeah… a pretty pervasive change.

@mbostock mbostock merged commit 424dd32 into main Sep 24, 2021
@mbostock mbostock deleted the mbostock/string-coercion branch September 24, 2021 14:32
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.

stringify should toString instead of valueOf
2 participants