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

Question: unreachable code in ES5.js #2

Closed
Xotic750 opened this issue Jan 3, 2016 · 3 comments
Closed

Question: unreachable code in ES5.js #2

Xotic750 opened this issue Jan 3, 2016 · 3 comments

Comments

@Xotic750
Copy link

Xotic750 commented Jan 3, 2016

In ES5.js, the following line is unreachable.

https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L47

Which in turn, it seems that ES5internalSlots is never called.

https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L10

And IsCallable is loaded but never used.

https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L7

Is this intentional?

@ljharb
Copy link
Owner

ljharb commented Jan 3, 2016

Hmm, good question. It's not intentional.

In the spec, ToPrimitive points to https://es5.github.io/#x8.12.8, so it actually appears like lines https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L40-L46 should be deleted.

However, it's also possible that using String and Number like I'm doing bypasses the need to implement [[DefaultValue]], which would mean I could delete it entirely, along with isCallable.

@ljharb
Copy link
Owner

ljharb commented Jan 4, 2016

Turns out I had a bunch of incorrect stuff in my ES5 tests. Fixing it, and removing https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L40-L46, plus one minor fix in https://github.com/ljharb/es-to-primitive/blob/master/es5.js#L11-L28, has the correct tests passing.

ljharb added a commit that referenced this issue Jan 4, 2016
…y primitive value, regardless of hint.

Discovered per #2.
@ljharb ljharb closed this as completed in 38d5ad1 Jan 4, 2016
@Xotic750
Copy link
Author

Xotic750 commented Jan 4, 2016

Thanks, I came across it when I started looking at using uglify's compress option, it started throwing warnings. Happy New Year! :)

ljharb added a commit to ljharb/es-abstract that referenced this issue Jan 5, 2016
ljharb added a commit that referenced this issue Sep 6, 2017
…y primitive value, regardless of hint.

Discovered per #2.
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

No branches or pull requests

2 participants