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: use lodash _baseIsNative directly instead of _.isNative (#12358) #12475
Conversation
to avoid core-js thrown error when core-js has been loaded.
Codecov Report
@@ Coverage Diff @@
## master #12475 +/- ##
=======================================
Coverage 96.37% 96.38%
=======================================
Files 95 95
Lines 9254 9255 +1
Branches 61 61
=======================================
+ Hits 8919 8920 +1
Misses 322 322
Partials 13 13
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for tests for this, there is already a test covering this. It will fail whenever native toString
overrides custom defined toString
I moved the comment down. I just noticed in the readme it says to target bug fixes to master first, do you want me to re-point this PR? |
Thanks, this PR is already pointed to |
Pull Request check-list
Please make sure to review and check all of these items:
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Description of change
Closes #12358
Lodash throws an error when calling _.isNative if core-js has been loaded because some of its shims can interfere with lodash's ability to accurate determine if a particular function is a native function or not. Using the _baseIsNative module directly bypasses the core-js check in lodash without requiring a custom implementation if the native function detection.
I didn't add any new unit test to validate that an error isn't thrown when core-js shims are loaded because I wasn't sure if you wanted an extra dev dependency for that. I also wasn't sure since Mocha doesn't isolate each test file to a separate process if you wanted the core-js loaded for all tests since that could in theory mask some other problem when core-js is not present.