Remove unneeded dependencies in favor of native varients - #23
Conversation
ljharb
left a comment
There was a problem hiding this comment.
Removing dependencies does not inherently improve supply chain security, first of all.
Second, a breaking change is insanely unacceptable, and this entire PR eliminates a very intentional category of robustness. I'm literally never going to accept changes like this in virtually any project I maintain.
| /** @type {typeof defaultReplacer} */ | ||
| var replacer = opts && opts.replacer | ||
| // eslint-disable-next-line no-extra-parens | ||
| ? /** @type {typeof defaultReplacer} */ (Function.call.bind(/** @type {any} */ (opts.replacer))) |
There was a problem hiding this comment.
the call-bind package is more robust than Function.call.bind, and its use is intentional to support browsers that don't have .bind.
| }; | ||
|
|
||
| if (isArray(node)) { | ||
| if (Array.isArray(node)) { |
There was a problem hiding this comment.
we support browsers that don't have Array.isArray.
| } | ||
|
|
||
| if ($indexOf(seen, node) !== -1) { | ||
| if (seen.indexOf(node) !== -1) { |
There was a problem hiding this comment.
this violates robustness; we shouldn't break if someone deletes indexOf off the builtin prototype.
|
Removing dependencies does not inherently improve supply chain security, first of all. Second, a breaking change is insanely unacceptable. |
This PR significantly improves supply chain security by replacing usage of multiple dependencies with native equivalents. This results in the removal of 22 dependencies (4 direct). The bundle size is now 21.7% of what is was before, an almost 5x reduction in package size. Performance has not changed at all, I ran multiple perfomance tests.