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

primitive wrapper objects cause unexpected behavior #140

Open
dwhieb opened this issue Aug 10, 2020 · 2 comments · Fixed by #151
Open

primitive wrapper objects cause unexpected behavior #140

dwhieb opened this issue Aug 10, 2020 · 2 comments · Fixed by #151
Labels
bug Bugs

Comments

@dwhieb
Copy link
Member

dwhieb commented Aug 10, 2020

this.#abbreviation = new String(val);

Describe the bug
Using a primitive wrapper object (new String) here causes unexpected behavior, because unlike normal instantiation of primitives, the wrapper object is not discarded after instantiation. In the code above, typeof this.#abbreviation will yield object rather than string.

Fix
The above code should either be converted back to the primitive value this.#abbreviation = (new String(val)).valueOf(), or just removed this.#abbreviation = val;. Probably the latter.

Reference

@dwhieb dwhieb added the bug Bugs label Aug 10, 2020
@dwhieb
Copy link
Member Author

dwhieb commented Aug 17, 2020

@dwhieb
Copy link
Member Author

dwhieb commented Sep 3, 2020

@vadekh My brain kept drifting back to this issue, and I finally realized why. I remembered that previously I had read that it's a great idea to use primitive object constructors like String() and Number() when doing type conversions because it makes the conversion explicit, and I even remembered doing this in previous projects. But I also remembered the various unexpected side effects of using primitive object wrappers, like the ones we ran into in our code. What I forgot is that there's a difference between new String(), which creates a new primitive object wrapper and returns an Object, and just String(), which is explicitly designed to do type conversion:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/String

Let's update our codebase so that we use String() etc. wherever we're doing explicit type conversion. There are probably multiple places in our code this would be applicable to now.

@dwhieb dwhieb reopened this Sep 3, 2020
@dwhieb dwhieb removed their assignment Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant