using utility methods for value type checking #16318
Conversation
|
Thanks for your contribution! The pull request is marked to be |
pissang
requested review from
100pah,
susiwen8,
plainheart and
Ovilia
and removed request for
100pah
src/scale/helper.ts
Outdated
|
|
||
| const mathLog = Math.log; | ||
| export const mathLog = Math.log; | ||
|
|
There was a problem hiding this comment.
I ever thought if it may be slightly better to extract all the mathematic functions to a common helper. Any idea? If it's feasible, that'll be a huge job.
There was a problem hiding this comment.
Removed this export.
I ever thought if it may be slightly better to extract all the mathematic functions to a common helper
I'm not sure about it. It will be helpful if some mathematic functions needs to be polyfilled(e.g. Math.hypot) or implentated. But If we only export the methods that exits in the Math ns. It seems to be too tricky to reduce the code size.
There was a problem hiding this comment.
@plainheart Seems d3 does the similar thing:
https://github.com/d3/d3-geo/blob/main/src/math.js#L21
There was a problem hiding this comment.
@pissang It should reduce some redundant Math, degree to radian, and other commonly used values like 2π, 1/2π to shrink the bundle size. But this may be insignificant.
|
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
Brief Information
This pull request is in the type of:
What does this PR do?
This PR replaces all
typeof xxx === 'string',typeof xxx === 'number',typeof xxx === 'function'toisString,isNumber,isFunctionutility methods to have neater codes. It reduces 1565 bytes in total after minimized.And the related benchmark about this change: https://jsbench.me/yskxzuqb9t/1 . Changing inline code to function call won't have performance drop. The interesting thing is some optimizations we did before like caching the typeof result in https://github.com/apache/echarts/blob/master/src/data/helper/dataValueHelper.ts#L158 will lead to worse performance on the chrome.
This PR also removes circular dependency introduced in #16300