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] SameValue: Use Object.is(…) if available #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Jun 7, 2020

Object.is(…) simply invokes the SameValue abstract operation.

Copy link
Owner

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This changes SameValue.js from 23 bytes to 366 bytes, a 16x increase. I'm not sure this change is particularly useful here - this function is trivial for engines to inline, and falling back to the builtin one adds unreliability for what i suspect is no performance gain.

If there's actual benchmarks that indicate that the native Object.is method performs significantly faster than this version, then it'd be worth the bundle size increase, but otherwise I think this is probably fine as-is.

@codecov
Copy link

codecov bot commented Jun 13, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.13%. Comparing base (0e7f6df) to head (b2f87e7).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
- Coverage   95.39%   92.13%   -3.26%     
==========================================
  Files        2140     2140              
  Lines       31289    31300      +11     
  Branches     7047     7058      +11     
==========================================
- Hits        29847    28838    -1009     
- Misses       1442     2462    +1020     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb ljharb force-pushed the fix/same-value/prefer-object-is branch from 69d241d to def7e76 Compare January 5, 2021 19:24
@ljharb ljharb force-pushed the fix/same-value/prefer-object-is branch from def7e76 to 9470189 Compare October 30, 2022 21:00
@ljharb ljharb force-pushed the fix/same-value/prefer-object-is branch from 9470189 to 8735417 Compare October 30, 2022 21:02
@ExE-Boss ExE-Boss force-pushed the fix/same-value/prefer-object-is branch from 8735417 to b2f87e7 Compare January 4, 2025 01:19
@ExE-Boss ExE-Boss closed this Jan 4, 2025
@ExE-Boss ExE-Boss reopened this Jan 4, 2025
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.

3 participants